Merge lp://staging/~cprov/adt-cloud-worker/uci-nova-refix into lp://staging/~canonical-ci-engineering/adt-cloud-worker/uci-nova

Proposed by Celso Providelo
Status: Merged
Approved by: Celso Providelo
Approved revision: 11
Merged at revision: 9
Proposed branch: lp://staging/~cprov/adt-cloud-worker/uci-nova-refix
Merge into: lp://staging/~canonical-ci-engineering/adt-cloud-worker/uci-nova
Diff against target: 59 lines (+9/-9)
1 file modified
uci-nova (+9/-9)
To merge this branch: bzr merge lp://staging/~cprov/adt-cloud-worker/uci-nova-refix
Reviewer Review Type Date Requested Status
Paul Larson Approve
Francis Ginther Approve
Evan (community) Needs Fixing
Review via email: mp+256740@code.staging.launchpad.net

Commit message

Fixing broken stderr/stdout redirect notation.

Description of the change

Fixing broken stderr/stdout redirect notation.

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

I'd like to hear more about what was wrong with this, perhaps a comment is in order to keep someone from trying the same thing again? +1 otherwise though

review: Approve
10. By Celso Providelo

Re-fixing uci-nova redirect notation (2>&1 instead 2&>1).

Revision history for this message
Joe Talbott (joetalbott) wrote :

looks okay to me. One in-line question.

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

One question

review: Needs Information
Revision history for this message
Evan (ev) wrote :

See inline.

review: Needs Fixing
11. By Celso Providelo

Addressing review comments.

Revision history for this message
Celso Providelo (cprov) wrote :

Ev, Paul,

Thanks for catching that problem, uci-nova is fixed.

Revision history for this message
Francis Ginther (fginther) wrote :

Tested with atd-run.

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

I'm actually still not 100% sure of the logic there, and it looks like you did something different from what Evan recommended but Francis said he was able to test it and see it work too. So, I'd like to see it land if it works, but maybe just add a quick comment about what it's doing there since there seems to be some confusion about it.

review: Approve
Revision history for this message
Celso Providelo (cprov) wrote :

Paul, diff line 55 is exactly what Ev suggested, I am missing something ?

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

Nope, you're correct. I think I ended up looking at an old revision because I was viewing the comments.

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