Bug 27114

Summary: bugzilla-tool: Parse short bug URL from commit log messages
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, mrowe
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch v1 aroben: review+

Description David Kilzer (:ddkilzer) 2009-07-09 05:46:08 PDT
Created attachment 32513 [details]
Patch v1

Reviewed by NOBODY (OOPS!).

* Scripts/bugzilla-tool:
(bug_id_from_commit_message): Check for the short bug URL before
checking for the longer bugs.webkit.org URL.
---
 2 files changed, 16 insertions(+), 1 deletions(-)
Comment 1 Adam Roben (:aroben) 2009-07-09 09:02:53 PDT
Comment on attachment 32513 [details]
Patch v1

I didn't even know we had these short URLs!

r=me
Comment 2 Eric Seidel (no email) 2009-07-09 14:41:43 PDT
I'd eventually like to shove all this stuff in a changelogs.py module which can export real ChangeLogEntry classes which know how to get bug_id, reviewer, date, description, etc. out of ChangeLog entries, and provide nice generators for iterating over ChangeLog entries in for in loops.

This looks good though.
Comment 3 David Kilzer (:ddkilzer) 2009-07-09 14:47:49 PDT
(In reply to comment #2)
> I'd eventually like to shove all this stuff in a changelogs.py module which can
> export real ChangeLogEntry classes which know how to get bug_id, reviewer,
> date, description, etc. out of ChangeLog entries, and provide nice generators
> for iterating over ChangeLog entries in for in loops.

I was thinking the same thing--a class to manage ChangeLog entries.  I may do that soon.
Comment 4 David Kilzer (:ddkilzer) 2009-07-09 18:01:06 PDT
$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/bugzilla-tool
Committed r45686

http://trac.webkit.org/changeset/45686
Comment 5 Eric Seidel (no email) 2009-07-09 18:07:45 PDT
It appears as though you still commit manually.  I'm wondering if there is something we should do to fix bzt land-diff for you?
bzt land-commits might work as well (although it doesnt' know how to fix up ChangeLogs for you).
Comment 6 David Kilzer (:ddkilzer) 2009-07-09 19:34:28 PDT
(In reply to comment #5)
> It appears as though you still commit manually.  I'm wondering if there is
> something we should do to fix bzt land-diff for you?
> bzt land-commits might work as well (although it doesnt' know how to fix up
> ChangeLogs for you).

The "land-commits" command doesn't exit currently.  I'm actually working on an implementation now.

For patches that I write, I have a different workflow than simply downloading patches from bugs and applying them.  I'm working on updating bugzilla-tool as I go--I can't fix it all in one commit, or I'll be asked to break it into pieces.  :)

I want my workflow to work like this (suggestions welcome for improvement):

1. Branch master.
2. Write the patch.
3. Run prepare-ChangeLog.
  * prepare-ChangeLog should add "<http://webkit.org/b/BUGID>" to description line, with "BUGID" to be replaced later.
4. Commit the result to the local branch.
5. Run bugzilla-tool create-bug branch-name.
6. [Optional] Update local commit as needed from review.
7. [Optional] Repost patch using bugzilla-tool post-commit branch-name.
8. Run bugzilla-tool land-commits -b NNNNN branch-name.
  * It should use the existing commit log message from git.
  * It should replace the "BUGID" text in both the commit log message and each ChangeLog.
  * It should replace the reviewer ("NOBODY (OOPS!)") text in both the commit log message and each ChangeLog.
  * It should close the bug like "land-diff" does when finished.
Comment 7 Eric Seidel (no email) 2009-07-09 19:47:49 PDT
Yeah, bugzilla tool was not really designed with committers in mind.  It was initially written exclusively so I could keep up with the Chromium commit queue (we have an insane number of contributers w/o commit rights it seems).  So it's designed to commit patches from bugzilla, and not from locally.

The following works OK if you're using git and want to commit something locally:

1.  git checkout my-branch-with-changes
2.  git svn rebase
4.  resolve-ChangeLog (of some form)
3.  git reset --soft trunk
4.  bugzilla-tool land-diff bugnumber
Comment 8 Eric Seidel (no email) 2009-07-09 19:50:31 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > It appears as though you still commit manually.  I'm wondering if there is
> > something we should do to fix bzt land-diff for you?
> > bzt land-commits might work as well (although it doesnt' know how to fix up
> > ChangeLogs for you).
> 
> The "land-commits" command doesn't exit currently.  I'm actually working on an
> implementation now.

Oh yeah.  Sorry, forget which ideas got finished and which didnt...

> For patches that I write, I have a different workflow than simply downloading
> patches from bugs and applying them.  I'm working on updating bugzilla-tool as
> I go--I can't fix it all in one commit, or I'll be asked to break it into
> pieces.  :)

Yup. :)  As I noted above, it wasn't really designed for committers originally.  It was designed to be run from a cron job as a commit-bot.

> I want my workflow to work like this (suggestions welcome for improvement):
> 
> 1. Branch master.
> 2. Write the patch.
> 3. Run prepare-ChangeLog.
>   * prepare-ChangeLog should add "<http://webkit.org/b/BUGID>" to description
> line, with "BUGID" to be replaced later.
> 4. Commit the result to the local branch.
> 5. Run bugzilla-tool create-bug branch-name.
> 6. [Optional] Update local commit as needed from review.
> 7. [Optional] Repost patch using bugzilla-tool post-commit branch-name.
> 8. Run bugzilla-tool land-commits -b NNNNN branch-name.
>   * It should use the existing commit log message from git.
>   * It should replace the "BUGID" text in both the commit log message and each
> ChangeLog.
>   * It should replace the reviewer ("NOBODY (OOPS!)") text in both the commit
> log message and each ChangeLog.
>   * It should close the bug like "land-diff" does when finished.

Seems fine.

Ideally bugzilla-tool will also be fixed for git users to instead of enforcing a clean tree, instead just stash local changes and make a new trunk branch when doing things like land-patches.
Comment 9 David Kilzer (:ddkilzer) 2009-07-09 19:59:00 PDT
(In reply to comment #7)
> The following works OK if you're using git and want to commit something
> locally:
> 
> 1.  git checkout my-branch-with-changes
> 2.  git svn rebase
> 4.  resolve-ChangeLog (of some form)
> 3.  git reset --soft trunk
> 4.  bugzilla-tool land-diff bugnumber

That's too manual.  :)

(In reply to comment #8)
> Ideally bugzilla-tool will also be fixed for git users to instead of enforcing
> a clean tree, instead just stash local changes and make a new trunk branch when
> doing things like land-patches.

You also can't assume which branch you're on.  I suspect we'd want to:

1. If local changes, stash them.
2. If not master branch, save current branch name and switch to master.
3. Update master and land patch.
4. Switch back to branch from Step #2 if not master.
5. Apply stashed changes from Step #1 if there were local changes.