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: Add sort-imports ignoreDeclarationSort (fixes #11019) #11040

Merged
merged 2 commits into from Jan 4, 2019

Conversation

Projects
None yet
6 participants
@remcohaszing
Copy link
Contributor

remcohaszing commented Oct 30, 2018

This allows users to enforce sorting import members, without having to enforce
sorting of import declarations.

fixes #11019

Update: Add sort-imports ignoreDeclarationSort (fixes #11019)
This allows users to enforce sorting import members, without having to enforce
sorting of import declarations.
@nzakas

nzakas approved these changes Dec 20, 2018

Copy link
Member

nzakas left a comment

LGTM. We do need a couple other reviews in order to merge.

@platinumazure
Copy link
Member

platinumazure left a comment

Requested some small documentation changes, but the code and tests look good to me. Thanks!

Show resolved Hide resolved docs/rules/sort-imports.md Outdated
Show resolved Hide resolved docs/rules/sort-imports.md Outdated
@@ -136,6 +138,34 @@ import c from 'baz.js';

Default is `false`.

### `ignoreDeclarationSort`

Ignores the sorting of import declaration statements.

This comment has been minimized.

@platinumazure

platinumazure Dec 22, 2018

Member

Please add a note that the default is false here. (I see the note below, but I think it's important enough to be included in the intro/summary of the section.)

This comment has been minimized.

@remcohaszing

remcohaszing Dec 29, 2018

Contributor

I decided to keep it at the end for consistency with the other sections.

Please let me know if this really needs to be changed.

@platinumazure
Copy link
Member

platinumazure left a comment

LGTM, thanks for explaining the rationale. I don't like the location of that info still but it can be fixed in a separate PR. I have no objections to this landing as is. Thanks!

@not-an-aardvark
Copy link
Member

not-an-aardvark left a comment

LGTM, thanks!

@btmills

btmills approved these changes Jan 4, 2019

Copy link
Member

btmills left a comment

LGTM, thank you!

@btmills btmills merged commit 0d91e7d into eslint:master Jan 4, 2019

5 checks passed

commit-message PR title follows commit message guidelines
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
release-monitor No patch release is pending
Details
```js
/*eslint sort-imports: ["error", { "ignoreDeclarationSort": false }]*/
import 'foo.js'
import 'bar.js'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment