novu icon indicating copy to clipboard operation
novu copied to clipboard

feat: added import-order for eslint

Open venarius opened this issue 1 year ago • 8 comments

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...) This adds the import-order rule for eslint to have package imports above relational imports, etc...

venarius avatar Sep 28 '22 11:09 venarius

@scopsy it is handled automatically using "eslint --fix" - I added the rule for it to the eslintrc

venarius avatar Sep 28 '22 14:09 venarius

This rule rullez :D I do use it often, but with a small addition, I do divide those imports also with an additional empty line, because sometimes they like merge together and it's tough to catch at first look. Also I do move imports from the same repository to the bottom of the "dependencies" imports part, like: first external dependencies, then same repo dependencies, then empty line and then relative. Yes, it saves time, increases readability :)

LetItRock avatar Sep 30 '22 19:09 LetItRock

I love the import order only for packages and local imports. But when it's ordered also by name, it's becoming very annoying.

nevo-david avatar Oct 01 '22 04:10 nevo-david

I love the import order only for packages and local imports. But when it's ordered also by name, it's becoming very annoying.

Is that because it's annoying to read or having to always fix it when eslint is complaining about the order? Because the later will be automatically handled by lint-staged here when commiting any changes.

venarius avatar Oct 01 '22 18:10 venarius

This PR is a bit stale. I am happy to fully review it but after agreeing with the rules for the imports, as it is too big to review it multiple times. My take on order import (and preference) is:

// Novu packages imports
// Runtime dependency imports

// Absolute path imports from current file

// Relative path imports from current file

Alphabetical order based on package/path name for me is helpful. But flexible in that.

p-fernandez avatar Oct 10 '22 10:10 p-fernandez

Was just about to merge when the list of merge conflicts :P

@venarius should we maybe take a look at it again after hacktoberfest? Because of the amount of PR's right now it will be hard to get it up to sync with outside collaborators. WdyT?

scopsy avatar Oct 16 '22 08:10 scopsy

Was just about to merge when the list of merge conflicts :P

@venarius should we maybe take a look at it again after hacktoberfest? Because of the amount of PR's right now it will be hard to get it up to sync with outside collaborators. WdyT?

Yeah, sure thing!

venarius avatar Oct 16 '22 14:10 venarius

@venarius do you want to give it another go? :)

scopsy avatar Nov 29 '22 12:11 scopsy