Launchpad itself

Merge lp://qastaging/~brian-murray/launchpad/bug-320596 into lp://qastaging/launchpad

bug-320596
Merge into devel

Proposed by Brian Murray on 2010-07-22

Status:

Merged

Approved by:

Brian Murray on 2010-08-26

Approved revision:

no longer in the source branch.

Merged at revision:

11458

Proposed branch:

lp://qastaging/~brian-murray/launchpad/bug-320596

Merge into:

lp://qastaging/launchpad

Diff against target:

362 lines (+176/-112)

5 files modified

lib/lp/bugs/interfaces/bugtarget.py (+120/-108)
lib/lp/bugs/interfaces/bugtask.py (+2/-2)
lib/lp/bugs/stories/webservice/xx-bug-target.txt (+2/-2)
lib/lp/bugs/tests/test_bugtask.py (+9/-0)
lib/lp/bugs/tests/test_searchtasks_webservice.py (+43/-0)

To merge this branch:

bzr merge lp://qastaging/~brian-murray/launchpad/bug-320596

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Abel Deuring (community)	code		Approve on 2010-08-26
Robert Collins (community)		2010-07-22	Needs Fixing on 2010-07-22
Review via email: mp+30690@code.qastaging.launchpad.net

Commit message

For the devel version of the API have omit_targeted searchTasks parameter default to False.

Description of the change

Remove the default value of True for omit_targeted so that bug tasks targeted to a milestone are no longer hidden in search results.

Tests modified:

bin/test -cvvt xx-bug.txt -t bugtask-search.txt

Revision history for this message

Robert Collins (lifeless) wrote on 2010-07-22:

So, I have some needs fixing and a question.

needs fixing: please don't add these things to doctests: none of your tests look like documentation, so they shouldn't be expressed as documentation.

question: looks like you're breaking the 1.0 API to me, I thought that wasn't permitted.

review: Needs Fixing

Revision history for this message

Brian Murray (brian-murray) wrote on 2010-07-23:

I think having the default value of omit_targeted be False in the API is actually broken. This is a different default value than the web interface and resulted in terribly confusing behavior - see bug 320596 (and its duplicates). However, I'll special case the default value of it if you think that is best.

Revision history for this message

Robert Collins (lifeless) wrote on 2010-07-23:

I don't hold the value to be sacred; however its my understanding that
we promised not to change the behaviour of the 1.0 API - we can change
devel, just not 1.0.

Revision history for this message

Brian Murray (brian-murray) wrote on 2010-08-20:

This is ready for review again. omit_targeted is left defaulting to True and in devel the default value is changed to False.

Revision history for this message

Abel Deuring (adeuring) wrote on 2010-08-26:

Hi Brian,

nice work! Just one minor nitpicks: I find the name "modifiable_params" and "modified_params" a bit confusing. What about "params_for_api_1_0" and "params_for_devel"? And since these variables are defined on mudole level, perhaps even "searchtasks_params_for_api_1_0"? (Having modified_params and modifiable_params for a second exported method might lead to funny results ;)

review: Approve (code)