Snapcraft

Merge lp://qastaging/~mvo/snapcraft/yaml-tests into lp://qastaging/~snappy-dev/snapcraft/core

  1. yaml-tests
  2. Merge into core
Proposed by Michael Vogt
Status: Merged
Approved by: Michael Terry
Approved revision: 95
Merged at revision: 84
Proposed branch: lp://qastaging/~mvo/snapcraft/yaml-tests
Merge into: lp://qastaging/~snappy-dev/snapcraft/core
Diff against target: 428 lines (+240/-76)
10 files modified
bin/snapcraft (+34/-0)
runtests.sh (+3/-3)
snapcraft/dirs.py (+30/-0)
snapcraft/main.py (+52/-63)
snapcraft/plugin.py (+2/-1)
snapcraft/tests/__init__.py (+25/-0)
snapcraft/tests/test_cmds.py (+4/-4)
snapcraft/tests/test_plugin.py (+4/-4)
snapcraft/tests/test_yaml.py (+84/-0)
snapcraft/yaml.py (+2/-1)
To merge this branch: bzr merge lp://qastaging/~mvo/snapcraft/yaml-tests
Reviewer Review Type Date Requested Status
Michael Terry (community) Approve
Review via email: mp+264555@code.qastaging.launchpad.net

Commit message

Add tests for snapcraft/yaml.py and some code reorg refactor.

Description of the change

Small branch that starts adding unittests around the snapcraft/yaml.py code.

To post a comment you must log in.
Revision history for this message
Michael Terry (mterry) wrote :

Thanks for the branch, mvo! Comments inline.

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for the review and for spotting my mistakes :) I replied inline and hope I addressed the issues you raised.

Revision history for this message
Michael Terry (mterry) wrote :

Awesome work, thanks. A comment inline about a way to avoid the symlink.

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for the review! I commented inline, I hope it addresses the points you raised, please let me know if there is more that can/should be done. I'm really happy with your feedback, I think this made the MP much nicer.

Revision history for this message
Michael Terry (mterry) wrote :

A couple inline comments and I think we're missing dirs.py

Revision history for this message
Michael Terry (mterry) wrote :

But this looks really nice already :)

Revision history for this message
Michael Terry (mterry) wrote :

And thanks for cleaning up the project layout. :)

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks again! *cough* I added the missing dirs.py now and also removed that unneeded file.

Revision history for this message
Michael Terry (mterry) wrote :

Alright, great! I still don't like the underscore, but won't hold up the rest of this goodness for that. Approved, thanks!

review: Approve
Revision history for this message
Michael Vogt (mvo) wrote :

Ups, I overlooked the comment about _intergration-tests. Silly me. This should be fixed now as well :)

Revision history for this message
Michael Vogt (mvo) wrote :

(Please let me know if I have overlooked any other comment or there is anything else that should be fixed)

Revision history for this message
Michael Terry (mterry) wrote :

No, that was the only one :)

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