Skip to content
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

Undocument --bes_best_effort #14333

Closed
wants to merge 4 commits into from

Conversation

Yannic
Copy link
Contributor

@Yannic Yannic commented Nov 25, 2021

We've heared from several people that they tried passing the flag and it did
not have any efect (as expected from a no-op), so let's undocument the flag
so it's no longer discoverable.

On a related note, we should document --bes_upload_mode which seems to be
what people want to do.

We've heared from several people that they tried passing the flag and it did
not have any efect (as expected from a no-op), so let's undocument the flag
so it's no longer discoverable.

On a related note, we should document `--bes_upload_mode` which seems to be
what people want to do.
@google-cla google-cla bot added the cla: yes label Nov 25, 2021
@Yannic
Copy link
Contributor Author

Yannic commented Nov 25, 2021

cc @lfpino

Copy link
Contributor

@lfpino lfpino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just delete this flag at this point. It's been a no-op for years.

@Yannic
Copy link
Contributor Author

Yannic commented Dec 6, 2021

Removing sounds tricky to me, looks like lots of builds would breaks because people are setting it. If we can cherry-pick to 5.x, we should be able to remove in 6.0

@Yannic
Copy link
Contributor Author

Yannic commented Dec 15, 2021

@meisterT I can do that, but that graveyard seems Bazel specific. Can you verify that moving it there won't break Blaze (or take care of also adding it to the Blaze graveyard when merging)?

@meisterT
Copy link
Member

Yep, that works for both Blaze and Bazel.

@Yannic
Copy link
Contributor Author

Yannic commented Dec 15, 2021

Done, PTAL

@Yannic Yannic mentioned this pull request Dec 24, 2021
9 tasks
@Yannic
Copy link
Contributor Author

Yannic commented Jan 5, 2022

What's the status of this? I'd like to get this into 5.0

/cc @Wyverald

@Wyverald
Copy link
Member

Wyverald commented Jan 7, 2022

@meisterT started the import process last year, but he's still on vacation until next week. In the meantime, @michaeledgar left the following comment:

I think this needs to go in AllCommandGraveyardOptions, because BES options are applied to many commands: see BazelBuildEventServiceModule#allowedCommands() and BlazeBuildEventServiceModule#allowedCommands().

@Yannic
Copy link
Contributor Author

Yannic commented Jan 26, 2022

Sorry for the delay, done. PTAL

@Yannic
Copy link
Contributor Author

Yannic commented Feb 16, 2022

Any updates here? @meisterT @michaeledgar

@Yannic
Copy link
Contributor Author

Yannic commented Mar 14, 2022

ping? I'd like to get this into 5.1

@Wyverald
Copy link
Member

I'll check with @meisterT

@meisterT
Copy link
Member

Sorry for the delay, giving it a kick internally.

@bazel-io bazel-io closed this in 18c04a2 Mar 17, 2022
@meisterT
Copy link
Member

@Yannic will you create the cherrypick against the release branch?

@Wyverald
Copy link
Member

I can do it.

Wyverald pushed a commit to Wyverald/bazel that referenced this pull request Mar 17, 2022
We've heared from several people that they tried passing the flag and it did
not have any efect (as expected from a no-op), so let's undocument the flag
so it's no longer discoverable.

On a related note, we should document `--bes_upload_mode` which seems to be
what people want to do.

Closes bazelbuild#14333.

PiperOrigin-RevId: 435317406
brentleyjones pushed a commit to brentleyjones/bazel that referenced this pull request Mar 17, 2022
We've heared from several people that they tried passing the flag and it did
not have any efect (as expected from a no-op), so let's undocument the flag
so it's no longer discoverable.

On a related note, we should document `--bes_upload_mode` which seems to be
what people want to do.

Closes bazelbuild#14333.

PiperOrigin-RevId: 435317406

Co-authored-by: Yannic Bonenberger <yannic@engflow.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants