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

Replace sorting plugin with sort keys #70

Merged
merged 2 commits into from
Mar 6, 2017

Conversation

nunofgs
Copy link
Contributor

@nunofgs nunofgs commented Mar 2, 2017

This PR replaces the sorting-plugin with the sort-keys rule which is available since eslint 3.3.0.

The motivation for this change is to respect the team's natural sort order requirement. Also, it's one less dependency (yay!).

Example that did not trigger an error with sorting-plugin (and was not configurable):

{
  var1: 'foo',
  var10: 'foo',
  var2: 'foo',
  var9: 'bar'
};

Using the new sort-keys rule, the object would have to be written as:

{
  var1: 'foo',
  var2: 'foo',
  var9: 'bar',
  var10: 'foo'
};

😌

@nunofgs nunofgs force-pushed the bugfix/replace-sorting-plugin-with-sort-keys branch from 9c270ff to 074d45b Compare March 2, 2017 16:00
@nunofgs nunofgs removed the wip label Mar 2, 2017
Copy link
Contributor

@rplopes rplopes left a comment

Choose a reason for hiding this comment

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

Missing test updates in test/fixtures/correct.js

@nunofgs nunofgs force-pushed the bugfix/replace-sorting-plugin-with-sort-keys branch from 074d45b to 0ba2794 Compare March 3, 2017 11:55
[`${sortObjectProps1}`]: 'foo',
[`${sortObjectProps2}`]: 'bar'
var9: 'bar',
var10: 'foo'
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT about adding a var1 as well, to test the natural order? Otherwise, both tests could pass with a non-natural descending order.

@nunofgs nunofgs force-pushed the bugfix/replace-sorting-plugin-with-sort-keys branch from 0ba2794 to 6b77c16 Compare March 6, 2017 18:43
@rplopes rplopes merged commit 1604f28 into master Mar 6, 2017
@rplopes rplopes deleted the bugfix/replace-sorting-plugin-with-sort-keys branch March 6, 2017 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants