Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: add --env-info flag to CLI #12270

Merged
merged 10 commits into from Sep 25, 2019
Merged

New: add --env-info flag to CLI #12270

merged 10 commits into from Sep 25, 2019

Conversation

kaicataldo
Copy link
Member

@kaicataldo kaicataldo commented Sep 16, 2019

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[x] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

fixes #11958

What changes did you make? (Give an overview)
#12060 appears to have stalled, so I took some time to clean up my original proof of concept and make a PR! This feels like a win for issue triaging, since we can ask for the output of this command in our bug issue templates.

Is there anything you'd like reviewers to focus on?
Nothing in particular.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Sep 16, 2019
@kaicataldo kaicataldo added accepted There is consensus among the team that this change meets the criteria for inclusion cli Relates to ESLint's command-line interface feature This change adds a new feature to ESLint and removed triage An ESLint team member will look at this issue soon labels Sep 16, 2019
Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great in general. Thanks for putting this together!

My only thought is that I'm not sure I'm a fan of the module name info. What info, exactly? Environment info? Rule info? Some other info? I think a name like environment-info or runtime-info would be a lot better even if it's a little more verbose.

@kaicataldo
Copy link
Member Author

runtime-info makes sense to me!

@kaicataldo
Copy link
Member Author

@platinumazure Updated 😃

@kaicataldo
Copy link
Member Author

kaicataldo commented Sep 17, 2019

I've also added some regression tests with Windows-style paths. There isn't any OS-specific path handling logic yet, but thought it would be a good idea to ensure that future changes work with both styles of paths.

@kaicataldo
Copy link
Member Author

Hmmm the Windows tests pass locally on my Windows machine but are failing in CI. Not sure why that is - something to do with the check to see if the executing version is global or not. I've removed that commit for now and will see if I can figure out what's going on. I do think these tests are adequate.

@kaicataldo
Copy link
Member Author

In the spirit of @platinumazure's comment, I'm wondering if we should actually call the flag --env-info. info is very generic, and this flag outputs specific information. It would probably be best to reserve info for a more fitting CLI flag. Thoughts?

@kaicataldo kaicataldo changed the title New: add --info flag to CLI New: add --env-info flag to CLI Sep 17, 2019
Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

lib/shared/runtime-info.js Show resolved Hide resolved
lib/shared/runtime-info.js Show resolved Hide resolved
tests/lib/shared/runtime-info.js Show resolved Hide resolved
tests/lib/shared/runtime-info.js Show resolved Hide resolved
@kaicataldo
Copy link
Member Author

Thanks for the reviews! I'm going to go ahead and merge this.

@kaicataldo kaicataldo merged commit a7894eb into master Sep 25, 2019
@kaicataldo kaicataldo deleted the info-flag branch September 25, 2019 20:45
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Mar 24, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Mar 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion cli Relates to ESLint's command-line interface feature This change adds a new feature to ESLint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cli]: Have a dedicated option to show up local environment information
4 participants