Ubuntu Developer Tools

Merge lp://staging/~nacc/ubuntu-dev-tools/update-vcs into lp://staging/~ubuntu-dev/ubuntu-dev-tools/trunk

update-vcs
Merge into trunk

Proposed by Nish Aravamudan on 2016-10-19

Status:

Needs review

Proposed branch:

lp://staging/~nacc/ubuntu-dev-tools/update-vcs

Merge into:

lp://staging/~ubuntu-dev/ubuntu-dev-tools/trunk

Diff against target:

241 lines (+129/-44)

2 files modified

ubuntutools/update_maintainer.py (+122/-43)
update-maintainer (+7/-1)

To merge this branch:

bzr merge lp://staging/~nacc/ubuntu-dev-tools/update-vcs

Related bugs:

Bug #567629: [update-maintainer] should adjust Vcs-foo fields too	Wishlist	In Progress
Bug #1595744: Tooling (or process) doesn't handle VCS- fields	Low	Triaged

Link a bug report

Reviewer	Date Requested	Status
Mattia Rizzolo		Needs Information on 2017-04-30
Benjamin Drung	2016-10-19	Needs Fixing on 2017-03-29
Review via email: mp+308871@code.staging.launchpad.net

Description of the change

I am not necessarily requesting this for merging yet, but it's a PoC implementation that resolves a process issue we hit somewhat often on the Server Team.

I am open to:

  - Making this a separate command
  - Adding a new command (update-metadata?) that update-maintainer aliases to for BC which supports the new flag (perhaps without the flag and by default therefore?)
  - Adding a parameter to --vcs to specify a new value should be used instead (similar for vcs-browser I guess?). Could use the protocol of the repository to determine the protocol to use for the tag, if needed?
  - ... any other changes, I just think code speaks louder than words :)

Revision history for this message

Benjamin Drung (bdrung) wrote on 2017-03-29:

A quick review.

1) Instead of duplicating code (including classes), please re-use ubuntutools.update_maintainer and add your new methods to the Control class.

2) The functions update_maintainer and restore_maintainer could gain an vcs parameter to update the vcs option or you can refactor the code to retrieve the control class and then run update_maintainer, update_vcs on it.

3) Having an option to set the vcs values would be nice

I don't thinks that splitting this feature into a separate command makes sense.

review: Needs Fixing

Revision history for this message

Nish Aravamudan (nacc) wrote on 2017-03-31:

I think I have addressed 1) and 2). I need to do some more testing and want to actually write some tests before it gets merged.

Revision history for this message

Mattia Rizzolo (mapreri) wrote on 2017-04-30:

I think the current code is fine now.
@nacc do you plan to also write some tests? It would be really nice to have some.

I do not think things like the point 3 @bdrung should stop the merge (but you are welcome to implement it of course!).

Another thing: you are closing bug #1595744 in usd-importer with this, is that really fine?

review: Needs Information

Revision history for this message

Nish Aravamudan (nacc) wrote on 2017-05-01:

Hi Mattia,

Yes, I plan on writing tests still. Feel free to hold off on merging
until I get to that!

The importer bug is correct to close (it was a user request to have
this feature in `update-maintainer`, which we use in the server team's
documented git-based merge workflow). But I did update it to have a
task for ubuntu-dev-tools specifically, so I can update the snap of
the importer to fix that task, once this fix is avaiable.

On Sun, Apr 30, 2017 at 10:05 AM, Mattia Rizzolo <email address hidden> wrote:
> Review: Needs Information
>
> I think the current code is fine now.
> @nacc do you plan to also write some tests? It would be really nice to have some.
>
> I do not think things like the point 3 @bdrung should stop the merge (but you are welcome to implement it of course!).
>
> Another thing: you are closing bug #1595744 in usd-importer with this, is that really fine?
> --
> https://code.launchpad.net/~nacc/ubuntu-dev-tools/update-vcs/+merge/308871
> You are the owner of lp:~nacc/ubuntu-dev-tools/update-vcs.

Revision history for this message

Mattia Rizzolo (mapreri) wrote on 2018-09-03:

@nacc any update here?

Revision history for this message

Mattia Rizzolo (mapreri) wrote on 2019-07-20:

@nacc Could you please finish this up? :)

Unmerged revisions

1456. By Nish Aravamudan on 2017-03-31

Fix obvious syntax errors. Sorry!

1455. By Nish Aravamudan on 2017-03-31

Pass verbose down to the internal functions.

1454. By Nish Aravamudan on 2017-03-31

Drop unnecessary whitespace change.

1453. By Nish Aravamudan on 2017-03-31

Merge with lp:ubuntu-dev-tools

1452. By Nish Aravamudan on 2017-03-31

Refactor code based upon MR feedback. Remove duplication of code between
update_maintainer.py and update_vcs.py. Delete update_vcs.py. Add a vcs
parameter to {update,restore}_maintainer to indicate if VCS fields
should be modified. Refactor both functions to not continue in the loop
until both maintainer and VCS fields have been examined if necessary.

1451. By Nish Aravamudan on 2016-10-19

update-maintainer: add --vcs mode

As we perform merges on the server team, we tend to try and replay the
maintainer changes to debian/control via update-maintainer. However, for
those packages that also update the Vcs fields, we have no automated
tooling to do the same. Add a fairly simple version of the same code
as in update_maintainer, but handling Vcs-Git-* fields (including
Browser).

LP: #567629, LP: #1595744

Signed-off-by: Nishanth Aravamudan <email address hidden>