Merge lp://staging/~adeuring/charm-tools/check-maintainer-branch-owner into lp://staging/~charmers/charm-tools/stable

Proposed by Abel Deuring
Status: Rejected
Rejected by: Abel Deuring
Proposed branch: lp://staging/~adeuring/charm-tools/check-maintainer-branch-owner
Merge into: lp://staging/~charmers/charm-tools/stable
Diff against target: 357 lines (+215/-25)
4 files modified
charmtools/__init__.py (+2/-0)
charmtools/proof.py (+92/-23)
setup.py (+1/-2)
tests/proof/test_proof.py (+120/-0)
To merge this branch: bzr merge lp://staging/~adeuring/charm-tools/check-maintainer-branch-owner
Reviewer Review Type Date Requested Status
charmers Pending
Review via email: mp+188586@code.staging.launchpad.net

Description of the change

This branch adds a check for the maintainer name and the branch owner.

The main idea is that maintainer(s) as specified in metadata.yaml
should also be members of the team that owns the related Launchpad
branch.

I thought a bit if it makes sense to find the intended target
branch of the proofed branch: In theory, it is possible to look for
the parent branch, the "grandparent" branch etc until an LP branch
for a sourcepackage of the "charms" distribution is found. That _could_
be the the final merge target for the given branch, but that's not
always the case -- somebody might create a fork of another branch
with the intention to maintain this fork for a longer time -- and this
is probably the main use case for the new check: To ensure that the
maintainer mentioned in metadata.yaml has write access to the LP
branch of the charm.

Another option would have been to find a push branch on LP and to check
for merge proposals, to eventually find the "right" target. But that is
also not trivial -- and it does not work if the proofed branch has not
yet been pushed to Launchpad.

I other words: Lots of cumbersome "guess work" with limited success
probabilty.

So I took the most simple approach: The user must explicitly
specify the merge target of the given branch.

Currently, the proofer checks if the name of the main charm directory
matches the charm name. This does not make sense for branches, so this
required some changes how the directory name is handled in function run().

I found the distinction between charm_name (a directory path) and
charm_base_name a bit confusing, so I renamed charm_name to charm_path
and charm_basename to charm_name.

It seems that we do not have any infrastructure for unit tests of
functions that access the LP web API, so I wrote also a little mock
for the LP API.

To post a comment you must log in.
252. By Abel Deuring

Make the version of charmtools available in the package itself.

Revision history for this message
Abel Deuring (adeuring) wrote :

Another change: I made the package version available in the package itself. The branch changes the parameter passed to the function run() of the proofer. With Charmworld, there is at least one other project that uses the proofer. Having a simple way to discover the version of charmtools makes it easier for charmworld to figure out how to call proof.run().

Unmerged revisions

253. By Abel Deuring

reverted all changes except definition of the version number in charmtools/__init__.py

252. By Abel Deuring

Make the version of charmtools available in the package itself.

251. By Abel Deuring

Check consistency of branch owner and maintainers specified in metadata.yaml

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: