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

Document ScheduleTaskRequest.serialized_task #7975

Merged
merged 1 commit into from
Nov 27, 2024
Merged

Document ScheduleTaskRequest.serialized_task #7975

merged 1 commit into from
Nov 27, 2024

Conversation

vanja-p
Copy link
Contributor

@vanja-p vanja-p commented Nov 27, 2024

No description provided.

@vanja-p vanja-p requested a review from bduffany November 27, 2024 21:07
Comment on lines +338 to +339
// Serialized build.bazel.remote.execution.v2.ExecutionTask. That proto
// depends on this one, so using the full type would create a build cycle.
Copy link
Member

Choose a reason for hiding this comment

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

nit: we could move ExecutionTask to scheduler.proto if we wanted to break this cycle, so the note about the dependency cycle feels a bit incidental. (We created this problem ourselves by tacking on ExecutionTask to the upstream API protos rather than adding our own package.) It might make sense to omit that part to avoid confusion, or maybe add some additional clarification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clarified a bit more.

@vanja-p vanja-p merged commit 1834b3c into master Nov 27, 2024
12 of 15 checks passed
@vanja-p vanja-p deleted the vanja-doc branch November 27, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants