Review: Approve -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Ursula Junque wrote: > Ursula Junque has proposed merging lp:~ursinha/bzr-pqm/add-noqa-incr-lpland > into lp:bzr-pqm. > > Requested reviews: > Bzr-pqm-devel (bzr-pqm-devel) > Related bugs: > #602137 add bzr lp-land support to no-qa and incr QA tags > https://bugs.launchpad.net/bugs/602137 > > > = Summary = > > This branch is a fix for bug 602137, adding to bzr lp-land three options: > --testfix, --no-qa and --incr. --no-qa option should add the [no-qa] tag to > the commit message, --incremental option should add the [incr] tag, and > --testfix, [testfix]. This enforces the QA policy of now allowing fixes to be > committed without bugs unless explicitely pointed. > > == Proposed fix == > > The implementation is pretty simple. A method checks if the --no-qa, > --incremental or --testfix options are given to bzr lp-land, and combined > with the bugs found in the mp decides if the merge can continue based on the > QA policy: > > * If the mp has no linked bugs and no no-qa tag, ec2 land should fail > because without bugs and no-qa tag that commit would be an orphaned one. > * If the mp has bugs and the no-qa commit tag, that's ok. The bug #s and the > no-qa tag will be added to the commit message. This will be properly parsed > and handled by qa-tagger script after the branch landed. > * If the mp has no linked bugs but has the no-qa commit tag, that's ok. The > commit will be qa-untestable, therefore not orphaned. > * If the mp has no linked bugs but has the incr commit tag, ec2 land should > fail, because bugs are needed when the fix is incremental. > * A commit cannot be no-qa and incr at the same time.
As far as reviewing goes, I'm going to trust that you know the Launchpad requirements more than I do. As such, I'm not really auditing this part. I'll just look at the code itself. Personally, I really don't like this notation: +__metaclass__ = type Certainly in bzrlib itself we always do: class Foo(object): instead. I'm certainly inclined to be less strict in bzr-pqm, esp. as I think the Launchpad standard way of doing it is with __metaclass__. (Also, I see that test_lpland was using __metaclass__ as well.) I'm going to just land this, but mostly I'm trusting that you've thought the logic through. Certainly everything I've seen seems well thought out. John =:-> merge: approve -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (Cygwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkxcPVQACgkQJdeBCYSNAAMDNQCcC6688GqhnisqHAjqVti2q3T/ L6kAnivByx3OiMqv8oDA4sV4ZTgosN0Z =zgkD -----END PGP SIGNATURE----- -- https://code.launchpad.net/~ursinha/bzr-pqm/add-noqa-incr-lpland/+merge/31703 Your team Bzr-pqm-devel is subscribed to branch lp:bzr-pqm. _______________________________________________ Mailing list: https://launchpad.net/~registry Post to : [email protected] Unsubscribe : https://launchpad.net/~registry More help : https://help.launchpad.net/ListHelp
