Merge lp://staging/~ev/charms/trusty/jenkins-remote-slave/shell-fixes into lp://staging/~canonical-ci-engineering/charms/trusty/jenkins-remote-slave/trunk

Proposed by Evan
Status: Merged
Approved by: Evan
Approved revision: 19
Merged at revision: 15
Proposed branch: lp://staging/~ev/charms/trusty/jenkins-remote-slave/shell-fixes
Merge into: lp://staging/~canonical-ci-engineering/charms/trusty/jenkins-remote-slave/trunk
Diff against target: 41 lines (+9/-8)
1 file modified
hooks/install (+9/-8)
To merge this branch: bzr merge lp://staging/~ev/charms/trusty/jenkins-remote-slave/shell-fixes
Reviewer Review Type Date Requested Status
Paul Larson Approve
Review via email: mp+237222@code.staging.launchpad.net

Commit message

Fix typo in the upstart job. Fix quoting of variables. Move from bash to dash; we don't need features above POSIX.

Description of the change

I first noticed that 'respawn' was typo'ed as 'arespawn.' Rather than make an MP for just that line, I rolled in a few other minor annoyances.

To post a comment you must log in.
Revision history for this message
Paul Larson (pwlars) wrote :

I think we should take a second look at the sed line, comments below. The resulting upstart job looks fine when I've tried it.

for the few extra lines we want, isn't the -x still useful? If not, I'm happy to remove it, I think it's worth having it there if something goes wrong.

18. By Evan

My mistake. The a was intentional.

19. By Evan

Keep line-by-line logging for now.

Revision history for this message
Evan (ev) wrote :

Ah, I missed that we were in append mode. Fixed.

Revision history for this message
Paul Larson (pwlars) wrote :

Looks good, +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: