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

Update: eslint --env-info output os info #14059

Merged
merged 3 commits into from Mar 20, 2021

Conversation

aladdin-add
Copy link
Member

@aladdin-add aladdin-add commented Feb 2, 2021

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an 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:

What changes did you make? (Give an overview)

eslint --env-info print the os info, it looks like:

Environment Info:

Node version: v15.5.0
npm version: v7.3.0
Local ESLint version: v7.19.0 (Currently used)
Global ESLint version: Not found
Operating System: darwin 20.3.0

Is there anything you'd like reviewers to focus on?

no.

@aladdin-add aladdin-add added enhancement This change enhances an existing feature of ESLint cli Relates to ESLint's command-line interface evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Feb 2, 2021
@mdjermanovic mdjermanovic added this to Evaluating in Triage Mar 8, 2021
@nzakas
Copy link
Member

nzakas commented Mar 9, 2021

I’m not sure it’s worth adding another dependency just to output debug information. Couldn’t we just use process.platform?

@aladdin-add
Copy link
Member Author

the package can output more readable info, e.g.

osName('win32', '6.3.9600');
//=> 'Windows 8.1'

I think either is fine. will remove the dependency if you prefer that way. :)

@nzakas
Copy link
Member

nzakas commented Mar 10, 2021

As I said, I just don’t think this is important enough to add another dependency. Win32 or Win64 is plenty of info for our purposes.

@aladdin-add
Copy link
Member Author

just rebased the latest master.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM

@mdjermanovic
Copy link
Member

I think we can already mark this as accepted, since it matches our new bug report template.

@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Mar 11, 2021
Comment on lines +61 to +62
os.platform = () => "darwin";
os.release = () => "20.3.0";
Copy link
Member

Choose a reason for hiding this comment

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

Can we use sinon here to prevent this change from leaking to other tests?

@mdjermanovic mdjermanovic changed the title Update: eslint --env-info ouput os info Update: eslint --env-info output os info Mar 20, 2021
Copy link
Member

@mdjermanovic mdjermanovic 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!

@mdjermanovic mdjermanovic merged commit 8984c91 into eslint:master Mar 20, 2021
Triage automation moved this from Evaluating to Complete Mar 20, 2021
@aladdin-add aladdin-add deleted the update/env-info branch April 22, 2021 00:52
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Sep 17, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 17, 2021
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 enhancement This change enhances an existing feature of ESLint
Projects
Archived in project
Triage
Complete
Development

Successfully merging this pull request may close these issues.

None yet

3 participants