search

bazel-contrib / target-determinator

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

Support -enforce-clean=enforce-clean for when the "before" location is unclean #85

Closed michaelboyd2 closed 6 months ago

michaelboyd2 commented 7 months ago

The -enforce-clean=enforce-clean option only seems to fail when the "after" location is unclean (when it is checked in cli/flags.go). In particular, it is not used in EnsureGitRepositoryClean in pkg/target_determinator.go. I think it probably should be? Perhaps another option could be added to -enforce-clean to activate this behaviour?

It would be great to understand from someone more familiar with target_determinator whether this is a good idea - I could potentially submit a PR if the direction were clear.

The surfaced for us when target_determinator identified all targets as changed when the "before" location was unclean (due to a .gitignore change).

illicitonion commented 6 months ago

Apologies for the delay, I've been away for a bit.

Yes, I agree that EnsureGitRepositoryClean should probably take a EnforceCleanFlag, and use that to drive whether to return an error. ResolveCommonConfig should probably not do this check at all, it should probably just be done by both EnsureGitRepositoryClean calls. I'd gladly review a PR which moves this around!

michaelboyd2 commented 6 months ago

@illicitonion could you take a look at #86? Not 100% sure this is the right fix. Thanks!