Merge lp://staging/~wgrant/launchpad/sensible-validate_target into lp://staging/launchpad
Proposed by
William Grant
Status: | Merged |
---|---|
Approved by: | Curtis Hovey |
Approved revision: | no longer in the source branch. |
Merged at revision: | 13514 |
Proposed branch: | lp://staging/~wgrant/launchpad/sensible-validate_target |
Merge into: | lp://staging/launchpad |
Diff against target: |
763 lines (+284/-248) 8 files modified
lib/canonical/launchpad/doc/validation.txt (+0/-105) lib/canonical/launchpad/interfaces/validation.py (+0/-98) lib/lp/bugs/browser/bugalsoaffects.py (+20/-22) lib/lp/bugs/browser/bugtask.py (+16/-13) lib/lp/bugs/browser/tests/test_bugtask.py (+2/-1) lib/lp/bugs/model/bugtask.py (+65/-7) lib/lp/bugs/model/tests/test_bugtask.py (+177/-0) lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt (+4/-2) |
To merge this branch: | bzr merge lp://staging/~wgrant/launchpad/sensible-validate_target |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email: mp+69175@code.staging.launchpad.net |
Commit message
Replace validate_distrotask and valid_upstreamtask with a more general and sensible validate_target.
Description of the change
This branch continues my ruthless campaign to move BugTask.target handling completely into the model. It takes validate_distrotask and valid_upstreamtask, combining them into a single validate_target which can be easily called by transitionToTarget. It now lives in lp.bugs, and has extensive unittests rather than doctests.
To post a comment you must log in.
This branch looks good to land. I have a comment that relates to my own recent work. Maybe your thoughts will help us to make a method faster.
> === modified file 'lib/lp/ bugs/model/ bugtask. py' bugs/model/ bugtask. py 2011-07-22 04:50:51 +0000 bugs/model/ bugtask. py 2011-07-25 23:06:42 +0000 target_ attribute( self, attr, value) target( bug, target): target) : urcePackage. providedBy( target) : sourcepackagena me is not None and distribution. series) > 0): distribution. guessPublishedS ourcePackageNam e( sourcepackagena me.name)
> --- lib/lp/
> +++ lib/lp/
...
> @@ -439,6 +437,65 @@
> return validate_
>
>
> +def validate_
> + """Validate a bugtask target against a bug's existing tasks.
> +
> + Checks that no conflicting tasks already exist.
> + """
> + if bug.getBugTask(
> + raise IllegalTarget(
> + "A fix for this bug has already been requested for %s"
> + % target.displayname)
> +
> + if IDistributionSo
> + # If the distribution has at least one series, check that the
> + # source package has been published in the distribution.
> + if (target.
> + len(target.
> + try:
> + target.
> + target.
I believe this is the method that contributes to bugs /bugs.launchpad .net/launchpad/ +bug/618366 and /bugs.launchpad .net/launchpad/ +bug/721643
https:/
https:/
I tried to avoid using it. Do you know why guessPublishedS ourcePackageNam e()
requires a string and does the extra work to get the spn. We already have
an spn half the time. I also want to avoid calling this method if the method
that provided the spn can be trusted to have provide one that was published
in the distro.
Ah, but this method is smart enough to verify the branch for the package.
I like that.
The binary name is in part the fault of the existing vocabs. I hope
Steven's work will make this path in the method obsolete, or at least very
rare.