Merge lp://staging/~cjohnston/ubuntu-ci-services-itself/reject-bad-lists into lp://staging/ubuntu-ci-services-itself

Proposed by Chris Johnston
Status: Merged
Approved by: Chris Johnston
Approved revision: 299
Merged at revision: 302
Proposed branch: lp://staging/~cjohnston/ubuntu-ci-services-itself/reject-bad-lists
Merge into: lp://staging/ubuntu-ci-services-itself
Diff against target: 152 lines (+73/-3)
4 files modified
cli/ci_libs/utils.py (+10/-0)
cli/tests/test_cli.py (+45/-2)
cli/tests/test_utils.py (+14/-1)
cli/ubuntu-ci (+4/-0)
To merge this branch: bzr merge lp://staging/~cjohnston/ubuntu-ci-services-itself/reject-bad-lists
Reviewer Review Type Date Requested Status
Chris Johnston (community) Approve
Andy Doan (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+208519@code.staging.launchpad.net

Commit message

Make the CLI check the formatting of -a and -r to ensure that there are no spaces

To post a comment you must log in.
Revision history for this message
Andy Doan (doanac) wrote :

don't want to sound like bike-shedding, but this confused me as I read. you have a function name:

21 +def is_valid_package_list(package_list_str):

but it returns true if it passes, and throws an exception if it fails. I think you want something like:

 def assert_valid_package_list(package_list_str)

the function returns nothing, but throws an exception if the list isn't correct.

My last comment is super shedding, so i'd just say ignore me. However, WRT "utils.InputError", I think just using Python's built-in exception "ValueError" might make more sense.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:297
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/276/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/276/rebuild

review: Approve (continuous-integration)
298. By Chris Johnston

Rename function to assert_valid_package_list, return nothing or raise error

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:298
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/279/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/279/rebuild

review: Approve (continuous-integration)
299. By Chris Johnston

Add a new unit test for checking an invalid entry

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:299
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/280/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/280/rebuild

review: Approve (continuous-integration)
Revision history for this message
Andy Doan (doanac) :
review: Approve
Revision history for this message
Chris Johnston (cjohnston) wrote :

 merge approved

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