Merge lp://staging/~abentley/launchpad/bad-branch-name into lp://staging/launchpad
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Māris Fogels | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 11015 | ||||
Proposed branch: | lp://staging/~abentley/launchpad/bad-branch-name | ||||
Merge into: | lp://staging/launchpad | ||||
Diff against target: |
141 lines (+55/-19) 3 files modified
lib/lp/code/browser/sourcepackagerecipe.py (+5/-0) lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+48/-19) lib/lp/code/model/sourcepackagerecipedata.py (+2/-0) |
||||
To merge this branch: | bzr merge lp://staging/~abentley/launchpad/bad-branch-name | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Māris Fogels (community) | Approve | ||
Review via email: mp+27549@code.staging.launchpad.net |
Commit message
Treat bad branch location in recipe as field error
Description of the change
= Summary =
Fix bug #592792: bad branch names in recipes produce oopses
== Proposed fix ==
Catch NoSuchBranch when creating the SourcePackageRecipe and set a suitable
field error.
== Pre-implementation notes ==
None
== Implementation details ==
Extracted createRecipe so it could be reused.
Implemented bad branch handling for SourcePackageRe
== Tests ==
bin/test -t test_create_
== Demo and Q/A ==
Attempt to create a recipe for a base branch that doesn't exist. This should
produce a validation error.
Attempt to create a recipe that merges a branchh that doesn't exist. This
should produce a validation error.
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
== Pyflakes notices ==
lib/lp/
283: local variable 'recipe_fings' is assigned to but never used
Hi Aaron,
This change looks good. Some questions:
* Would the text "No such branch: foo" be clearer for our users than "Unknown branch: foo"? I personally find the meaning of "No such X" clearer than "Unknown X".
* I see two convoluted calls to "extract_ text(find_ tags_by_ class(browser. contents, 'message')[1])" in this diff. Should it be extracted into a nice helper function like get_page_message()?
Otherwise, this looks good to me. r=mars
Maris