bazel-contrib / target-determinator

Determines which Bazel targets were affected between two git commits.
Apache License 2.0
144 stars 22 forks source link

--enforce-clean accepts a tri-state value but only has boolean behaviour #87

Open illicitonion opened 6 months ago

illicitonion commented 6 months ago

As far as I can tell, there is no code which differentiates between AllowIgnored and AllowDirty... @sitaktif do you remember any history here? AFAICT there has never been any code which differentiates between these, or any tests which set --enforce-clean=allow-dirty (or indeed which set --enforce-clean=allow-ignored)?

https://github.com/bazel-contrib/target-determinator/pull/86 is going to simplify this behaviour to be more boolean (but doesn't remove the three variants) - I think this is ok (and that we should just remove the extra unused variant), but figured I should file an issue to discuss...

sitaktif commented 6 months ago

Ouch, it does indeed look like I only implemented the --enforce-clean=allow-dirty path, and made --enforce-clean=allow-ignored act like --enforce-clean=allow-dirty.

I am happy to remove the allow-ignored state or maybe error out for now, so we can display something explanatory.