Merge lp://staging/~rockstar/launchpad/merge-queue-index into lp://staging/launchpad/db-devel
Status: | Merged |
---|---|
Approved by: | Gavin Panella |
Approved revision: | no longer in the source branch. |
Merged at revision: | 9943 |
Proposed branch: | lp://staging/~rockstar/launchpad/merge-queue-index |
Merge into: | lp://staging/launchpad/db-devel |
Diff against target: |
433 lines (+302/-3) 11 files modified
lib/lp/code/browser/branch.py (+6/-1) lib/lp/code/browser/branchmergequeue.py (+79/-0) lib/lp/code/browser/configure.zcml (+26/-0) lib/lp/code/browser/tests/test_branchmergequeue.py (+127/-0) lib/lp/code/model/branchmergequeue.py (+3/-0) lib/lp/code/templates/branch-pending-merges.pt (+15/-1) lib/lp/code/templates/branchmergequeue-index.pt (+39/-0) lib/lp/testing/__init__.py (+2/-0) lib/lp/testing/factory.py (+1/-1) setup.py (+2/-0) versions.cfg (+2/-0) |
To merge this branch: | bzr merge lp://staging/~rockstar/launchpad/merge-queue-index |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Approve | ||
Review via email: mp+39488@code.staging.launchpad.net |
Commit message
Add branch merge queue index page. Add soupmatchers library dependency.
Description of the change
This branch adds a (really basic) index page for branch merge queues, as long as a flow for creating a merge queue and linking a branch to it. This branch also adds some more things, including:
- The addition of james_w's soupmatchers library for HTML page matchers. This makes pagetest-type unit tests that we write in code a little nicer.
- Fixed a bug in getUserBrowser where the authentication didn't support any passwords but 'test'. I fixed this by setting the password for auth to the sooper sekrit attribute that holds the cleartext password.
- I also fixed a bug in the factory where the branch merge queues owner and registrant were being specified backwards.
This work is hiding behind a feature flag, and the UI is really raw, but I need to get this (and the next branch) landed so that others can also start working on branch merge queues.
Merge queues are cool. A few pretty minor comments.
[1]
+ def enable_ queue_flag( self): ().add( FeatureFlag( code.branchmerg equeue' ,
+ getFeatureStore
+ scope=u'default', flag=u'
+ value=u'on', priority=1))
There's a neat context manager in lp.testing that you might like. I
think the flag only needs to be enabled on the branch index page, so
something like this might work:
from lp.testing import feature_flags, set_feature_flag
with feature_flags():
set_feature_ flag(u' code.branchmerg equeue' , u'on') wser(
canonical_ url(branch) , user=rockstar)
browser = self.getUserBro
Okay, that's not actually much clearer. Anyway, it's there.
[2]
I think you should ad a short test to show that the +create-queue link
is not available when the feature flag is not set.
[3]
+ if configuration is None: simplejson. dumps({ }))
+ configuration = unicode(
The docstring for unicode says:
Create a new Unicode object from the given encoded string.
encoding defaults to the current default string encoding.
So I think this would be more correct as:
configuration = simplejson. dumps({ }).decode( 'ascii' )
Or:
configuration = simplejson. dumps({ }, ensure_ascii=False)
In practice it makes bugger all difference though because I doubt
Launchpad will ever run with a default encoding that isn't a superset
of ASCII.
I should just shut up :)
[4]
+ <h4>Merge Queue</h4>
+ This branch is not managed by a queue.
Maybe a test for this too.
[5]
+ password = naked_user. _password_ cleartext_ cached
This clobbers the password argument. Can you remove the argument? (In
a follow-on branch is fine if you need to get this landed.)