[Webkit-unassigned] [Bug 33197] bugzilla-tool submit-patch mistakenly picks up bug URLs in non-ChangeLog files

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=33197


David Kilzer (ddkilzer) <ddkilzer at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #46247|review?                     |review-
               Flag|                            |




--- Comment #6 from David Kilzer (ddkilzer) <ddkilzer at webkit.org>  2010-01-10 21:38:45 PST ---
(From update of attachment 46247)
> +        Also, fixed a wide-spread typo.

This should be a separate patch.  rs=me to fix the typo separately.

> diff --git a/WebKitTools/Scripts/webkitpy/commands/download.py b/WebKitTools/Scripts/webkitpy/commands/download.py
> @@ -268,6 +269,7 @@ Commits the revert and updates the bug (including re-opening the bug if necessar
>      @staticmethod
>      def _parse_bug_id_from_revision_diff(tool, revision):
>          original_diff = tool.scm().diff_for_revision(revision)
> +        # FIXME: This is wrong because it picks up bug numbers outside of ChangeLogs
>          return parse_bug_id(original_diff)

Why wasn't this instance fixed at the same time?  At minimum, this should be
explained in the ChangeLog.

> diff --git a/WebKitTools/Scripts/webkitpy/mock_bugzillatool.py b/WebKitTools/Scripts/webkitpy/mock_bugzillatool.py
> index f8392aa..ee5a3c8 100644
> --- a/WebKitTools/Scripts/webkitpy/mock_bugzillatool.py
> +++ b/WebKitTools/Scripts/webkitpy/mock_bugzillatool.py
> @@ -156,11 +156,14 @@ class MockSCM(Mock):
>      def commit_ids_from_commitish_arguments(self, args):
>          return ["Commitish1", "Commitish2"]
>  
> +    def commit_message_for_this_commit(self):
> +        return CommitMessage(["CommitMessage1", "https://bugs.webkit.org/show_bug.cgi?id=93"])
> +
>      def commit_message_for_local_commit(self, commit_id):
>          if commit_id == "Commitish1":
> -            return CommitMessage("CommitMessage1\nhttps://bugs.example.org/show_bug.cgi?id=42\n")
> +            return CommitMessage(["CommitMessage1", "https://bugs.webkit.org/show_bug.cgi?id=42"])
>          if commit_id == "Commitish2":
> -            return CommitMessage("CommitMessage2\nhttps://bugs.example.org/show_bug.cgi?id=75\n")
> +            return CommitMessage(["CommitMessage2", "https://bugs.webkit.org/show_bug.cgi?id=75"])
>          raise Exception("Bogus commit_id in commit_message_for_local_commit.")

Why didn't test results change in response to changing these text inputs? 
Shouldn't there be tests for grabbing the bug id from a local commit?  (Or to
reverse the question, why change the URLs if test results didn't change?)

r- to split the renames into a different patch and to answer the questions
above.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.