Introduce command that combines `check` and `detect-stale-violations`

[mclark]

Currently in our monolith we are running packwerk check in our CI. This is great and helps us manage our technical in a controlled manner. However, if a team removes some deprecated references and forgets to run packwerk update-deprecations there is no CI failure and the associated deprecated_references.yml file just gets a bit out of date with reality since we don't run packwerk detect-stale-violations in CI.

This only becomes a problem when other people start running packwerk update-deprecations and suddenly multiple PRs have the same line removals in them.

I propose one of the following ideas and I'm looking for thoughts and feedback about which approach might be most palatable:

• Update packwerk check to also detect stale violations
• Add packwerk check --include-stale to optionally change its return code based on stale violations
• Add a new command packwerk full-check or something similar that checks both

What do folks think? Is this something that others would find helpful?

[rafaelfranca]

Do we need to change or add a new command command? Can't you also run packwerk detect-stale-violations on CI?

[mclark]

Yes, we can definitely run the other check as a separate CI but it just seems wasteful to me. At scale, that's a lot of processing that doesn't really need to happen, we can probably make a worthwhile positive environmental impact by combining these two.

[rafaelfranca]

I agree, but at the same time if we combine both commands in our CI it would take more time than running all tests of the app, I'm surprised this is not the case in your app as well.

I'm ok with the inclusion of a new command though, but I'd prefer to change check to do both. Can you see how much the total time would increase if we change check to do both? If it is a lot we will need to create another command.

[mclark]

but at the same time if we combine both commands in our CI it would take more time than running all tests of the app, I'm surprised this is not the case in your app as well.

My thinking is that the parse will only have to happen once, meaning the call to stale_violations? will be the only addition to check and is relatively quick. That being said, I actually haven't verified that yet and may be overlooking something. I'll set up a branch and let you know.