[Webkit-unassigned] [Bug 27119] bugzilla-tool: Add create-bug command

[bugzilla-daemon at webkit.org]

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


David Levin <levin at chromium.org> changed:

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




--- Comment #18 from David Levin <levin at chromium.org>  2009-07-28 19:22:15 PDT ---
(From update of attachment 33683)
Biggest concern:

I don't understand the bootstrapping aspect to this. In order to create a well
formed patch, I need a changelog with a bug link/title.  This tool creates a
bug for a patch, but by definition, the patch must not be well formed because
the bug doesn't exist yet.

It could modify the ChangeLog right after creating the bug (but that sounds
complicated especially considering that you can given it a COMMITISH....)

On to the details.


> diff --git a/WebKitTools/Scripts/bugzilla-tool b/WebKitTools/Scripts/bugzilla-tool
> +class CreateBug(Command):
> +    def __init__(self):
> +        Command.__init__(self, 'Create a bug from local changes or local commits', '[COMMITISH]', options=options)

Nice to add period to this sentence.

> +
> +    def create_bug_from_commit(self, options, args, tool):
> +        commit_ids = tool.scm().commit_ids_from_range_arguments(args, cherry_pick=True)

This won't be very friendly to svn users if they happen to given an extra
argument.  Maybe it should check that it is git and if not, error out with a
friendly message.


> diff --git a/WebKitTools/Scripts/modules/bugzilla.py b/WebKitTools/Scripts/modules/bugzilla.py
> +    def check_create_bug_response_returning_bug_id_on_success(self, response_html):

If you don't expect external callers, then prefix it with a _

It is quite long. What about _get_create_bug_id(self, create_bug_response):
or something else (even) shorter?

> +    def create_bug_with_patch(self, bug_title, bug_description, component, patch_file_object, patch_description, cc, mark_for_review=False):
...
> +        if not component or component not in [item.name for item in component_items]:
> +            component = self.prompt_for_component([item.name for item in component_items])

This is fine.  I like this better:

        component_names = map(lambda item: item.name, component_items)
        if not component or component not in component_names:
            component = self.prompt_for_component(component_names)

You choose.

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