Page MenuHomePhabricator

Improve teamcity agent setup documentation and service script
ClosedPublic

Authored by Fabien on Mar 28 2019, 22:30.

Details

Summary

These minor improvements comes from my experience in setting up an agent
from scratch using this documentation.

Test Plan
Read the doc and setup an agent

Diff Detail

Repository
rABC Bitcoin ABC
Branch
teamcity_improve_doc
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5328
Build 8718: Bitcoin ABC Buildbot (legacy)
Build 8717: arc lint + arc unit

Event Timeline

jasonbcox requested changes to this revision.Mar 28 2019, 22:38
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
contrib/teamcity/README.md
22 ↗(On Diff #7862)

Does this really need to be part of this README? Installing sudo on a fresh machine seems intuitive enough and expected for any fresh machine you're setting up.

27 ↗(On Diff #7862)

adduser needs sudo (both lines)

This revision now requires changes to proceed.Mar 28 2019, 22:38
Fabien retitled this revision from Iprove teamcity agent setup documentation and service script to Improve teamcity agent setup documentation and service script.Mar 29 2019, 06:58
contrib/teamcity/README.md
22 ↗(On Diff #7862)

It depends on the goal of the document.
If this is intended to be a step by step guide, sudo needs to be installed as it is not shipped with Debian Stretch by default.
It is also an opportunity to write the adduser teamcity sudo to remain adding the user to the group.

If the goal of this document is to provide help for installing an agent on any linux, with Debian as an example and not a guide, I agree this is overkill.

Let me know what you think is the right approach here.

27 ↗(On Diff #7862)

Not yet, as the teamcity user is not in the sudo group these commands should be run when logged as root.
This is maybe not very clear from the comments, I will improve it.

Make clear what commands need to be run as root

jasonbcox added inline comments.
contrib/teamcity/README.md
22 ↗(On Diff #7862)

I suppose it's ok since the doc explicitly states a clean-slate VM.

27 ↗(On Diff #7862)

Ah, right. Thanks for fixing the comments.

This revision is now accepted and ready to land.Mar 29 2019, 16:08
This revision was automatically updated to reflect the committed changes.