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

Fix: revert changes to key-spacing due to regression #12598

Merged
merged 1 commit into from Nov 25, 2019
Merged

Conversation

@kaicataldo
Copy link
Member

kaicataldo commented Nov 25, 2019

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

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

What changes did you make? (Give an overview)
As reported in #12597, this change muddles the meaning of single line and multi line objects in a way that can lead to more errors being reported. This wasn't the intention, and I don't think we should be introducing more errors in a minor release for an autofix bug fix.

I'd like to advocate for reverting this change, releasing a patch release, and revisiting this (with @yeonjuan's help if they're willing!).

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

@eslint eslint bot added the triage label Nov 25, 2019
@kaicataldo kaicataldo changed the title Revert "Update: Fix uglified object align in key-spacing (fixes #1141… Fix: revert changes to key-spacing due to regression Nov 25, 2019
Copy link
Member

btmills left a comment

Agreed this is an unintended regression that deserves a patch.

kaicataldo added a commit that referenced this pull request Nov 25, 2019
Revert "Update: Fix uglified object align in key-spacing (fixes #11414) (#12472)"

This reverts commit 6503cb8.
@kaicataldo kaicataldo force-pushed the revert-12472 branch 2 times, most recently from 1c46144 to b9c0a48 Nov 25, 2019
Revert "Update: Fix uglified object align in key-spacing (fixes #11414) (#12472)"

This reverts commit 6503cb8.
@kaicataldo kaicataldo force-pushed the revert-12472 branch from b9c0a48 to e9c0584 Nov 25, 2019
@btmills btmills merged commit dd1e9f4 into master Nov 25, 2019
18 checks passed
18 checks passed
Verify Files
Details
Test (ubuntu-latest, 13.x)
Details
Test (ubuntu-latest, 12.x)
Details
Test (ubuntu-latest, 10.x)
Details
Test (ubuntu-latest, 8.x)
Details
Test (ubuntu-latest, 8.10.0)
Details
Test (windows-latest, 12.x)
Details
Test (macOS-latest, 12.x)
Details
Browser Test
Details
commit-message Commit message follows guidelines
Details
continuous-integration Build #20191125.6 succeeded
Details
continuous-integration (Test on Node.js 10 (Linux)) Test on Node.js 10 (Linux) succeeded
Details
continuous-integration (Test on Node.js 12 (Linux)) Test on Node.js 12 (Linux) succeeded
Details
continuous-integration (Test on Node.js 12 (Windows)) Test on Node.js 12 (Windows) succeeded
Details
continuous-integration (Test on Node.js 12 (macOS)) Test on Node.js 12 (macOS) succeeded
Details
continuous-integration (Test on Node.js 8 (Linux)) Test on Node.js 8 (Linux) succeeded
Details
licence/cla Contributor License Agreement is signed.
Details
release-monitor This change is semver-patch
Details
@btmills btmills deleted the revert-12472 branch Nov 25, 2019
@kaicataldo kaicataldo restored the revert-12472 branch Nov 26, 2019
@kaicataldo kaicataldo deleted the revert-12472 branch Nov 26, 2019
@kaicataldo kaicataldo restored the revert-12472 branch Nov 26, 2019
@kaicataldo

This comment has been minimized.

Copy link
Member Author

kaicataldo commented Nov 26, 2019

Weird - I had to restore the branch and delete it again to get this to disappear from my "open PRs" list...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.