-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use output directory naming scheme diff_against_dynamic_baseline
#18561
Conversation
@linzhp Could you test the change? I did and it seemed to resolve the issue. |
7e527b9
to
0e72491
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It worked. Thanks!
@sdtwigg for triage - do we still need this PR? |
Okay, it has been about a month since my change was submitted so users have had a chance to test and reported no issues (at least compared to the old diff_against_baseline). I think it might be reasonable to consider setting this as the new default. Any thoughts? PS: What is with the objc thing? Can you use the helper-function in CoreOptions instead of the raw equality check? |
Do you need a global blazerc change before this? We can discuss in the import |
Compared to `diff_against_baseline`, this mode improves caching when top-level flags change that are reset in the exec configuration.
0e72491
to
f0d06ba
Compare
Thanks for the pointer, I wasn't aware of the helper function. Changed.
As far as I know google3 already uses a different value than Bazel. If that's still the case this wouldn't be needed. |
This is correct. |
@sdtwigg Friendly ping, it would be great if we could get this into a rolling release soon so that it can enjoy testing "in the wild" well before the 7.0.0 branch cut. |
Compared to
diff_against_baseline
, this mode improves caching when top-level flags change that are reset in the exec configuration.Fixes #18480