Snappy

Merge lp://qastaging/~jamesodhunt/snappy/move-utility-functions into lp://qastaging/~snappy-dev/snappy/snappy-moved-to-github

move-utility-functions
Merge into snappy-moved-to-github

Proposed by James Hunt on 2015-02-03

Status:	Work in progress
Proposed branch:	lp://qastaging/~jamesodhunt/snappy/move-utility-functions
Merge into:	lp://qastaging/~snappy-dev/snappy/snappy-moved-to-github
Diff against target:	266 lines (+122/-106) 2 files modified partition/bootloader_uboot.go (+0/-106) partition/utils.go (+122/-0)
To merge this branch:	bzr merge lp://qastaging/~jamesodhunt/snappy/move-utility-functions
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Sergio Schvezov		2015-02-03	Needs Fixing on 2015-02-03
Review via email: mp+248340@code.qastaging.launchpad.net

Description of the change

* partition/bootloader_uboot.go: Move utility functions to utils.go.
* partition.utils.go: runCommandWithStdout(): Fix bug where an
additional empty line is included

Revision history for this message

Michael Vogt (mvo) wrote on 2015-02-03:

Thanks for this branch. Some comments inline (I copied them from https://code.launchpad.net/~jamesodhunt/snappy/add-lsblk-cache/+merge/248245 I hope I did not forgot a relevant one).

I also feel that the new functions in utils.go needs tests. It does not have to be done in this MP and I'm happy to merge this once my questions below are answered but I think we should add them soon.

Revision history for this message

Sergio Schvezov (sergiusens) wrote on 2015-02-03:

I added a couple of inline comments and have additional comments here:

* it doesn't seem hard to add tests, can you add them?
* it seems the original intention was to move this to a package itself, not a compilation unit, what made you make your mind up on putting it into the same package?

Thanks.

review: Needs Fixing

Revision history for this message

Sergio Schvezov (sergiusens) wrote on 2015-02-03:

One more inline comment about the output last line trim.

review: Needs Fixing

Revision history for this message

Sergio Schvezov (sergiusens) wrote on 2015-02-26:

I set this to work in progress to clear out the review queue

Unmerged revisions

135. By James Hunt on 2015-02-03: * partition/bootloader_uboot.go: Move utility functions to utils.go.
* partition.utils.go: runCommandWithStdout(): Fix bug where an
additional empty line is included in the output.