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

Breaking: remove deprecated browser/jest/node globals (fixes #10141) #10210

Merged
merged 1 commit into from Apr 13, 2018

Conversation

not-an-aardvark
Copy link
Member

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

[x] Other, please explain:

What changes did you make? (Give an overview)

This removes deprecated globals from the browser, jest, and node environments, as discussed in #10141.

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

Nothing in particular

This removes deprecated globals from the `browser`, `jest`, and `node` environments.
@not-an-aardvark not-an-aardvark added core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion breaking This change is backwards-incompatible labels Apr 12, 2018
@not-an-aardvark not-an-aardvark added this to Ready to merge in v5.0.0 Apr 12, 2018
@platinumazure
Copy link
Member

platinumazure commented Apr 13, 2018

Not that I'm strongly opposed, but I think the issue only really discussed browser/jest globals (although, I do see similar comments in the codebase around the node globals).

Do we need to reconfirm with TSC about the node globals? Were the node globals discussed in the meeting?

@not-an-aardvark
Copy link
Member Author

You're right -- due to an oversight on my part, I didn't notice that we also had deprecated node globals when I created the card (which got converted to an issue afterwards).

There wasn't any discussion about node globals at the meeting, although there also wasn't any substantial discussion about the other globals, either -- there was unanimous agreement among those present as soon as the issue was brought up, so we voted on it and moved onto the next issue.

The TSC question was "Should we remove the deprecated globals in ESLint 5.0?". Although the summary didn't mention deprecated node globals, I don't think there is a significant difference between removing node globals versus other globals, so I think it's reasonable to interpret the consensus as being in favor of removing all deprecated globals including the node globals.

Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

Though we didn't specifically discuss node globals, my assumption when I gave my 👍 to the change was that it would apply to all the globals marked as "For backward compatibility. Remove those on the next major release.", so this change matches my expectations.

tl;dr: LGTM

@kaicataldo kaicataldo merged commit a039956 into master Apr 13, 2018
v5.0.0 automation moved this from Ready to merge to Done Apr 13, 2018
@not-an-aardvark not-an-aardvark deleted the remove-deprecated-globals branch April 13, 2018 19:24
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Oct 11, 2018
@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 Oct 11, 2018
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 breaking This change is backwards-incompatible core Relates to ESLint's core APIs and features
Projects
No open projects
v5.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants