Merge lp://qastaging/~gesha/linaro-license-protection/1146220 into lp://qastaging/~linaro-automation/linaro-license-protection/trunk

Proposed by Georgy Redkozubov
Status: Merged
Approved by: Stevan Radaković
Approved revision: 186
Merged at revision: 184
Proposed branch: lp://qastaging/~gesha/linaro-license-protection/1146220
Merge into: lp://qastaging/~linaro-automation/linaro-license-protection/trunk
Diff against target: 131 lines (+85/-0)
2 files modified
scripts/publish_to_snapshots.py (+31/-0)
tests/test_publish_to_snapshots.py (+54/-0)
To merge this branch: bzr merge lp://qastaging/~gesha/linaro-license-protection/1146220
Reviewer Review Type Date Requested Status
Stevan Radaković Approve
Review via email: mp+160352@code.qastaging.launchpad.net

Description of the change

This branch reinstates BUILD-INFO.txt support for publishing service.
Publishing script checks if BUILD-INFO.txt is present among artifacts being published or among already published artifacts for the same build. Otherwise publishing is aborted with an error.

To post a comment you must log in.
Revision history for this message
Stevan Radaković (stevanr) wrote :

Nice work gesha, thanks!

Only thing I dislike here is having return values(PASS, FAIL) in the method instead of using the exceptions as this is a very good example on when they should be used. That way you don't need to return anything at all, just catch exception if the method throws it. Tests would need to go through a little change as well because of it.

review: Needs Fixing (code)
184. By Georgy Redkozubov

Rewritten code to use exceptions.

Revision history for this message
Stevan Radaković (stevanr) wrote :

46 @@ -387,6 +408,7 @@
47 if build_dir_path is None or target_dir_path is None:
48 print "Problem with build/target path, move failed"
49 return FAIL
50 + publisher.check_buildinfo(build_dir_path, target_dir_path)

Think we should have here something like:

try:
    publisher.check_buildinfo(build_dir_path, target_dir_path)
except as e:
    print e.strerror
    return FAIL

185. By Georgy Redkozubov

Rewrite exception handling.

186. By Georgy Redkozubov

Fix pep8

Revision history for this message
Stevan Radaković (stevanr) wrote :

Looks good.
Approve +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