Ubuntu system image

Merge lp://staging/~sil2100/ubuntu-system-image/server_si-nondestructive into lp://staging/ubuntu-system-image/server

  1. server_si-nondestructive
  2. Merge into server
Proposed by Łukasz Zemczak
Status: Needs review
Proposed branch: lp://staging/~sil2100/ubuntu-system-image/server_si-nondestructive
Merge into: lp://staging/ubuntu-system-image/server
Diff against target: 51 lines (+12/-4)
2 files modified
bin/copy-image (+6/-2)
bin/import-images (+6/-2)
To merge this branch: bzr merge lp://staging/~sil2100/ubuntu-system-image/server_si-nondestructive
Reviewer Review Type Date Requested Status
Barry Warsaw (community) Needs Information
Review via email: mp+260964@code.staging.launchpad.net

Commit message

Re-propose slightly modified changes from Caio. Add an option to not remove (clean-up) orphaned files during common script execution.

Description of the change

Re-propose slightly modified changes from Caio. Add an option to not remove (clean-up) orphaned files during common script execution.

To post a comment you must log in.
Revision history for this message
Barry Warsaw (barry) wrote :

On Jun 03, 2015, at 03:19 PM, Łukasz Zemczak wrote:

>Re-propose slightly modified changes from Caio. Add an option to not remove
>(clean-up) orphaned files during common script execution.

Can you add a test?

Reply
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

As in the previous merge request, I can attempt to add an unit test for this but since it's essentially a small change in the script and the scripts themselves are rather not good-enough units suited for unit testing by principle, this can look a bit strange. We also never had any unit tests for s-i bin scripts.

Reply
Revision history for this message
Barry Warsaw (barry) wrote :

Understood about the lack of unittests for the cli. Yes, this is a big tech-debt that we should eventually pay down.

Looking more closely at the change, it seems fine from visual inspection.

I have one concern about the ui though. I wonder if it would be better to call the option --keep-orphans. That's both more descriptive and avoid the double negatives.

review: Needs Information
Reply

Unmerged revisions

266. By Łukasz Zemczak

Merge trunk

265. By Łukasz Zemczak

Include a short help string for the no-cleanup option

264. By Łukasz Zemczak

Fix typo in help string.

263. By Łukasz Zemczak

Remove the additional safety-check as there does not seem to be any real reason for it right now

262. By Caio Begotti

turn the new behavior really new, respecting current "production deployments" of the code that might not be easily updated so anyone looking for the new behavior must explicetely pass the --no-cleanup option to copy or import scripts

261. By Caio Begotti

avoid being so destructive when running the script, as it might wipe out any artifacts in the same path of the publish path

the different from before is that to keep the old behaviour you need to pass option -c to the copy and import images commands, that is all

for the rest of us it means the default behaviour now is to never wipe out images after they are processed, this way image server can coexist with an archiver storage for instance, like capomastro needs in the PES team to build phone images

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