unused-code / unused

A tool to identify potentially unused code.
https://unused.codes
MIT License
283 stars 11 forks source link

[Feature Request] Exit code = 1 if Files Found (or Tokens Found) > 0 #22

Closed markaschneider closed 2 years ago

markaschneider commented 3 years ago

Exiting with a code other than 0 is commonly used as a trigger in git hooks to abort the action. For anyone who wants to abort a commit or push if there are unused tokens, this would be useful (instead of checking the output). And, thanks for writing this tool!

joshuaclayton commented 3 years ago

@markaschneider interesting idea!

Would you be up for talking this through a bit further? I think this is feasible to do, but it might be surprising to folks in certain cases. For example, there's three different removal likelihoods (high, medium, and low) – and everything would be at least low likelihood.

I could imagine using a 1 exit code for high-likelihood (which is what we display by default); in that situation, if there were no high-likelihood but there were medium-likelihood (again, not displayed by default), we'd return a 0 exit code?

Finally, how do you think we should handle exit codes for e.g. JSON output – same thing? And to confirm, a non-zero exit code wouldn't prevent folks from writing STDOUT to a file, correct?

joshuaclayton commented 3 years ago

Hi @markaschneider any additional thoughts to the questions I posed above?

mauromorales commented 3 years ago

I'd be interested in this feature too. I'm now using unused with Github Actions and would benefit from having a non 0 output if there are any findings for unused code.

I don't know Mark's need. But here are some ideas of what would work for me in case that's useful:

Depending on the flag I pass, it should know that that's the level of likelyhood that for me is a failure ... as you say, by default high-likelihood only but I can make it more sensitive by increasing the likelyhood

Additionally you could require to pass another flag that enforces the triggering of the non zero code in case you don't want to break it for existing users ... or as an experimental while you find the right UX

I'd expect the same that I expect from a test suite ... output is on STDOUT even if there's a failure and the exit code is non-zero

Thanks so much for this tool, it's been super useful!

mauromorales commented 3 years ago

Another way that I'd see this being quite useful is if I could configure unused to know of a certain unused methods that I don't want it to flag ... for example I'm going through a cleanup and there are methods I'm aware are not in use but I won't remove just yet, however I'd like that new PRs don't introduce new cases beyond those methods

joshuaclayton commented 2 years ago

@mauromorales I've opened up a PR with the behavior you'd mentioned above, hidden behind the --harsh flag.

Another way that I'd see this being quite useful is if I could configure unused to know of a certain unused methods that I don't want it to flag ... for example I'm going through a cleanup and there are methods I'm aware are not in use but I won't remove just yet, however I'd like that new PRs don't introduce new cases beyond those methods

Yes, this is a great point. There's currently a configuration file that can be managed and diverge from the stock configuration, but I haven't documented this in the readme.

mauromorales commented 2 years ago

@joshuaclayton thanks, I ended up finding about it while digging in the code ... we're using it on our production app and it works like a charm 👌

joshuaclayton commented 2 years ago

@mauromorales I've gone ahead and released 0.3.0 and updated the Homebrew formula to match. Mind following the upgrade instructions and try out the --harsh flag?

joshuaclayton commented 2 years ago

@mauromorales actually, I'd made a mistake in the CLI for detecting tokens 🤦🏻‍♂️ I'll try to get out a fix tomorrow and cut 0.3.1. Sorry for the confusion!

mauromorales commented 2 years ago

@joshuaclayton awesome, thanks for unused and for the heads up ... and no worries, I'll wait for 0.3.1 then

joshuaclayton commented 2 years ago

@mauromorales 0.3.1 is cut – if you follow the upgrade instructions I'd shared above, it should be all set!