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

Add eslint fix command to package.json as a convenience script for developers preparing pull requests. #9000

Open
fardarter opened this issue Apr 10, 2024 · 3 comments
Labels
Type: Improvement Make something better

Comments

@fardarter
Copy link
Contributor

Add eslint fix command to package.json as a convenience script for developers preparing pull requests.

Eg, "lint-fix": "eslint --color --cache . --fix"

@garethbowen
Copy link
Member

@fardarter I'm not a big fan of this approach because you end up with a whole lot of lint commands which is really complicated. Instead you can pass parameters directly to the command with the -- option, eg: npm run lint -- --fix.

The problem with this at the moment is the lint command actually does two things and the --fix gets appended so applied to the second thing and not the eslint command. The easy fix is to reverse the order of commands, eg:

    "lint": "./scripts/build/blank-link-check.sh && eslint --color --cache .",

This then also supports passing other parameters through to the eslint command. What do you think? Is this an easier solution?

@fardarter
Copy link
Contributor Author

My experience is that most developers don't know how to pass args to npm scripts. In general, I like to give devs easy tools to prepare a PR, as it makes my life easier when I have to give feedback. A preset script can be helpful there.

I also like keeping operations discrete and then bundling them into a sort of prepare PR command. This helps with speed, as people can make some fixes quickly, while still being able to run the overall script if they want.

Personally I like commit hooks but every team I've worked with has voted against them, and they do cause overhead, so I rather try make fixing things easier.

If you're still unpersuaded, then let's swap the calls around. Just not easy to leave a comment in .json to explain why the lint must always be at the end. Also, if you introduce other tooling you may end up with tooling conflicts if you actually need them to run after lint.

@garethbowen
Copy link
Member

In general, I like to give devs easy tools to prepare a PR

Definitely, but if you fill the package with scripts with various combinations of flags then developers can't find the tools and they're useless. Keep it simple!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement Make something better
Projects
None yet
Development

No branches or pull requests

2 participants