-
-
Notifications
You must be signed in to change notification settings - Fork 947
rename slave to worker #291
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
Conversation
buildworker.rst
Outdated
|
|
||
| We only allow builds to be triggered against commits to the CPython repository, | ||
| or committer-initiated branches hosted on hg.python.org. This means that the | ||
| or committer-initiated branches hosted on `https://github.com/python/cpython`_. This means that the |
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.
I made an assumption that this link should be updated to reference GitHub since that is where the code is now stored.
per brief discussion in #python-dev with berker.
zware
left a comment
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.
Looking good, but a few comments:
buildbots.rst
Outdated
| the buildbot fleet rather than just on Travis and AppVeyor. To do so, you can | ||
| make use of the `custom builders | ||
| <http://buildbot.python.org/all/waterfall?category=custom.stable&category=custom.unstable>`_. | ||
| <http://buildbot.python.org/all/#/waterfall?category=custom.stable&category=custom.unstable>`_. |
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.
This whole link doesn't really work anymore. We could try http://buildbot.python.org/all/#/builders?tags=custom.unstable&tags=custom.stable instead.
buildworker.rst
Outdated
|
|
||
| Running a buildslave | ||
| ==================== | ||
| Running a buildworker |
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.
"buildworker" doesn't really fit the terminology now used by Buildbot and doesn't flow as easily as "buildslave", either. Just "worker" fits Buildbot's new terminology, with "buildbot worker" for disambiguation where necessary. This applies pretty much everywhere in this PR, and it's a judgement call as to whether each instance needs disambiguation.
buildworker.rst
Outdated
|
|
||
| As for what kind of buildbot to run...take a look at our `current fleet | ||
| <http://buildbot.python.org/all/buildslaves>`_. Pretty much anything that isn't | ||
| <http://buildbot.python.org/all/#/builders>`_. Pretty much anything that isn't |
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.
This should be workers rather than builders.
buildworker.rst
Outdated
| want to run an OS that's already on our list there may be utility in setting it | ||
| up: we also need to build and test python under various alternate build | ||
| configurations. Post to the mailing list and talk about what you'd like to | ||
| up. We also need to build and test python under various alternate build |
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.
I agree that the colon isn't quite right, but I'd rather just replace it with a semicolon than a full stop. And the space doesn't need to be removed before Post.
</nitpick>
| For Windows: | ||
|
|
||
| * Create a buildbot user as a "standard" user. | ||
| * Install the latest version of Python 2.7 from python.org. |
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.
This no longer needs to specify 2.7; we can use any version we want now :)
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.
This series of instructions was a tad bit confusing so I specified that the last step is only necessary if using pip.
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.
I'm a little lost on your comment here. My comment was just that we can remove "2.7" from this line :)
buildworker.rst
Outdated
|
|
||
| mkdir buildarea | ||
| buildslave create-slave buildarea buildbot.python.org:9020 slavename slavepasswd | ||
| buildworker create-worker buildarea buildbot.python.org:9020 workername workerpasswd |
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.
s/buildworker/buildbot-worker/
This applies everywhere the buildslave command was used.
buildworker.rst
Outdated
| request that the master be restarted. | ||
|
|
||
| Latent slaves should also be updated periodically to include operating system | ||
| Latent worker should also be updated periodically to include operating system |
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.
s/worker/workers/
buildworker.rst
Outdated
|
|
||
| Latent slaves should also be updated periodically to include operating system | ||
| Latent worker should also be updated periodically to include operating system | ||
| or other software updates, but when do do such maintenance is largely up to you |
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.
s/do do/to do/ (please, I know it's not your typo :))
|
Thank you for the thorough review @zware. I made all the requested changes and used my best judgement when switching between "buildbot worker" and "worker". |
buildworker.rst
Outdated
| * Install either the bundle from python.org [#]_, or pip. | ||
| * Open a terminal window. | ||
| * Execute ``pip install buildbot-slave``. | ||
| * If using pip; execute ``pip install buildbot-worker``. |
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.
I would structure this as:
- Create a buildbot user ...
- Log in as the buildbot user
- Install buildbot, either from python.org or by running pip install buildbot-worker
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.
Thanks, this is much clearer.
conf.py
Outdated
| linkcheck_anchors_ignore = [ | ||
| # match any anchor that starts with a '/' since this is an invalid HTML anchor | ||
| '\/.*', | ||
| ] |
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.
Missing newline at the end of the file.
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.
Ugh, even my linter yelled at me and I ignored it. Thank you :)
|
Hey @zware I made all of the changes that you requested. Is there anything else I can do for this PR? |
zware
left a comment
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.
Just a couple of minor fixes to go :)
buildworker.rst
Outdated
| * Install either the Python 2.7 bundle from python.org [#]_, or pip. | ||
| * Open a terminal window. | ||
| * Execute ``pip install buildbot-slave``. | ||
| * Install the buildbot worker, either from python.org [#]_ or by running ``pip install buildbot-worker``. |
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.
This isn't quite right; buildbot-worker is not available from python.org. I'd say just take out everything between worker, and by.
| For Windows: | ||
|
|
||
| * Create a buildbot user as a "standard" user. | ||
| * Install the latest version of Python 2.7 from python.org. |
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.
I'm a little lost on your comment here. My comment was just that we can remove "2.7" from this line :)
|
Thank you @zware! I think we are all set. Just pushed a change which makes the MacOS section (hopefully) crystal clear. |
…onGH-291) Fixes pythonGH-290. Also makes linkcheck play nicely with the new buildbot.python.org.
This PR renames all references from "slave" to "worker".
It fixes all broken URLs related to buildbot.
In addition, I added:
to the
conf.pyfile. The link checker was complaining that things like/consolewere invalid anchors. This is true since these are dynamically generated and not valid HTML anchors.fix #290