Summary: | webkit-patch reads wrong bug url from unified diff context | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hajime Morrita <morrita> | ||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, commit-queue, ddkilzer, eric | ||||||
Priority: | P3 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Hajime Morrita
2010-03-22 21:27:32 PDT
Created attachment 51393 [details]
patch v0
Comment on attachment 51393 [details]
patch v0
This is the wrong fix. In this case you want to just use the code for pulling the bug from the ChangeLogEntry instead of the diff. This proposed change would still wrongly grab bug urls added in other parts of the diff besides the ChangeLog.
I recently added a ChangeLogEntry class which exposes a bug_id() method which should fix this case.
Thank you for the fix attempt though! I think we have at least one dup of this bug. Thank you for reviewing quickly! (In reply to comment #2) > This is the wrong fix. In this case you want to just use the code for pulling > the bug from the ChangeLogEntry instead of the diff. This proposed change > would still wrongly grab bug urls added in other parts of the diff besides the > ChangeLog. Agreed. > I recently added a ChangeLogEntry class which exposes a bug_id() method which > should fix this case. I couldn't find bug_id() on ChangeLogEntry. It looks not landed yet. Would you tell me which bug contains that? I'll watch it and retry this after its landing. BTW, Bug 33471 and Bug 33197 looks similar. But I'm not sure. Created attachment 52145 [details]
Patch
Comment on attachment 52145 [details]
Patch
I don't think it's right that commit_message_for_this_commit throws. But that's an old mistake of mine. Would be nice to have a FIXME in your try-catch, but this is fine as is.
Comment on attachment 52145 [details] Patch Clearing flags on attachment: 52145 Committed r56863: <http://trac.webkit.org/changeset/56863> All reviewed patches have been landed. Closing bug. |