Skip to content

Add readme notes for worker apis#290

Merged
bergsieker merged 2 commits intomainfrom
unknown repository
Mar 19, 2024
Merged

Add readme notes for worker apis#290
bergsieker merged 2 commits intomainfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Mar 11, 2024

Replaces #289
CLA should be happy now.

@ghost ghost requested a review from bergsieker as a code owner March 11, 2024 15:27
README.md Outdated
Comment on lines 71 to 79
Servers generally distribute work to a fleet of workers. While there is no
standard API for communication between the server and workers, links to the
APIs from some existing implementations are provided as a reference below.

The [Remote Worker API](https://docs.google.com/document/d/1s_AzRRD2mdyktKUj2HWBn99rMg_3tcPvdjx3MPbFidU)
defines a generic protocol for worker and server communication, although,
this API is considered too heavyweight for most use-cases.

*Adhering to any one of these protocols is not a requirement.*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I agree with all of the sentences under this heading, but I think that the order in which they are provided can be improved. What's your thought on the following?

Servers generally distribute work to a fleet of workers. The Remote Worker API defines a generic protocol for worker and server communication, although, this API is considered too heavyweight for most use-cases. Because of that, many implementations have designed their own protocols. Links to these APIs are provided as a reference below. Adhering to any one of these protocols is not a requirement.

  • List of protocols goes here.

Copy link
Author

Choose a reason for hiding this comment

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

I like it. Updated!

Copy link
Collaborator

@EdSchouten EdSchouten left a comment

Choose a reason for hiding this comment

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

@bergsieker PTAL!

@bergsieker bergsieker merged commit 96942a2 into bazelbuild:main Mar 19, 2024
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.

3 participants