fix(exit-code): Exit with status 1 when there are deprecated rules

sarbbottam / eslint-find-rules

Find built-in ESLint rules you don't have in your custom config

http://npm.im/eslint-find-rules

MIT License

206 stars 36 forks source link

fix(exit-code): Exit with status 1 when there are deprecated rules #287

Closed randycoulman closed 6 years ago

randycoulman commented 6 years ago

In #286, I added support for reporting on deprecated rules. According to the documentation and tests I wrote, eslint-find-rules is supposed to exit with code 1 when there are deprecated rules (unless the --no-error flag is also provided). However, in real usage, this didn't happen.

I discovered that the exit status assertions in the tests were not actually being called in all cases, resulting in the tests passing incorrectly. Looks like I forgot to make sure that the test failed for the right reason in this case. Mea culpa :-(.

I made the following changes:

Modify the tests to always capture the process exit code
Add assertions against the captured exit code where necessary
Re-work how processExitCode is managed in the code

ta2edchimp commented 6 years ago

Hmm ... @randycoulman could you please rebase on master? I am not able to push the merged master after rebasing the pr on it. Unless @ljharb has another idea — I basically wanted to ff merge the pr-branch, but of course the rebase altered the commit's hash, so it's unknown to github, thus being rejected.
Maybe I'm just not seeing something here ... will get back at it tomorrow, possibly it's already simply too late for me and the brain of mine for today ...

ta2edchimp commented 6 years ago

I feel dumb now ... may as well just press »rebase and merge« on the web ui.
How do we want to handle this in the future?

ljharb commented 6 years ago

@ta2edchimp you can rebase and force push onto the fork. i'll take care of it.

ljharb commented 6 years ago

done

ta2edchimp commented 6 years ago

Thanks a lot! I assumed to be lacking write permission to a fork, any fork. That's interesting.
I'll do the rest tomorrow, as well as preparing a release / raising the usual PR to bump the version.

ljharb commented 6 years ago

Did you clone using https or ssh? I’ve found that only using ssh solves lots of issues.

ta2edchimp commented 6 years ago

SSH. I have to admit, I never even tried to push to a fork / to a branch over at a fork during a PR because I would have assumed write permissions to be optionally per default.