Changeset 57552 in webkit


Ignore:
Timestamp:
Apr 13, 2010 5:23:39 PM (14 years ago)
Author:
ojan@chromium.org
Message:

2010-04-13 Ojan Vafai <​ojan@chromium.org>

Reviewed by David Levin.

Add experimental prototype Rietveld integration to webkit-patch upload
​https://bugs.webkit.org/show_bug.cgi?id=37418

This patch adds bare-bones integrtion with Rietveld for code reviews.
The behavior is hidden behind the --fancy-review command line flag.
Currently, there's no support for uploading more than one patch per
issue (which is a nice feature of Rietveld). The plan is to play with
this for a bit and see if it's useful.

Modified from Adam's original patch to autoinstall the rietveld upload script.

  • Scripts/webkitpy/common/config/init.py:
  • Scripts/webkitpy/common/net/rietveld.py: Added.
  • Scripts/webkitpy/common/net/rietveld_unitttest.py: Added.
  • Scripts/webkitpy/tool/commands/queues_unittest.py:
  • Scripts/webkitpy/tool/commands/upload.py:
  • Scripts/webkitpy/tool/commands/upload_unittest.py:
  • Scripts/webkitpy/tool/main.py:
  • Scripts/webkitpy/tool/mocktool.py:
  • Scripts/webkitpy/tool/steps/init.py:
  • Scripts/webkitpy/tool/steps/options.py:
  • Scripts/webkitpy/tool/steps/postcodereview.py: Added.
  • Scripts/webkitpy/tool/steps/postdiff.py:
Location:
trunk/WebKitTools
Files:
2 added
11 edited
1 copied

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r57551 r57552  
     12010-04-13  Ojan Vafai  <ojan@chromium.org>
     2
     3        Reviewed by David Levin.
     4
     5        Add experimental prototype Rietveld integration to webkit-patch upload
     6        https://bugs.webkit.org/show_bug.cgi?id=37418
     7
     8        This patch adds bare-bones integrtion with Rietveld for code reviews.
     9        The behavior is hidden behind the --fancy-review command line flag.
     10        Currently, there's no support for uploading more than one patch per
     11        issue (which is a nice feature of Rietveld).  The plan is to play with
     12        this for a bit and see if it's useful.
     13
     14        Modified from Adam's original patch to autoinstall the rietveld upload script.
     15
     16        * Scripts/webkitpy/common/config/__init__.py:
     17        * Scripts/webkitpy/common/net/rietveld.py: Added.
     18        * Scripts/webkitpy/common/net/rietveld_unitttest.py: Added.
     19        * Scripts/webkitpy/tool/commands/queues_unittest.py:
     20        * Scripts/webkitpy/tool/commands/upload.py:
     21        * Scripts/webkitpy/tool/commands/upload_unittest.py:
     22        * Scripts/webkitpy/tool/main.py:
     23        * Scripts/webkitpy/tool/mocktool.py:
     24        * Scripts/webkitpy/tool/steps/__init__.py:
     25        * Scripts/webkitpy/tool/steps/options.py:
     26        * Scripts/webkitpy/tool/steps/postcodereview.py: Added.
     27        * Scripts/webkitpy/tool/steps/postdiff.py:
     28
    1292010-04-13  Sam Weinig  <sam@webkit.org>
    230

  • trunk/WebKitTools/Scripts/webkitpy/common/config/__init__.py

    r56504 r57552  
    11# Required for Python to search this directory for module files
     2
     3import re
     4
     5codereview_server_host = "wkrietveld.appspot.com"
     6codereview_server_regex = "https?://%s/" % re.sub('\.', '\\.', codereview_server_host)
     7codereview_server_url = "https://%s/" % codereview_server_host

  • trunk/WebKitTools/Scripts/webkitpy/common/net/rietveld_unittest.py

    r57551 r57552  
    1 # Copyright (C) 2010 Google Inc. All rights reserved.
    2 # 
     1# Copyright (c) 2010 Google Inc. All rights reserved.
     2#
    33# Redistribution and use in source and binary forms, with or without
    44# modification, are permitted provided that the following conditions are
    55# met:
    6 # 
     6#
    77#     * Redistributions of source code must retain the above copyright
    88# notice, this list of conditions and the following disclaimer.
    … …  
    1414# contributors may be used to endorse or promote products derived from
    1515# this software without specific prior written permission.
    16 # 
     16#
    1717# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
    1818# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
    … …  
    2727# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
    2828
    29 import StringIO
     29import unittest
    3030
    31 from webkitpy.tool.steps.abstractstep import AbstractStep
    32 from webkitpy.tool.steps.options import Options
     31from webkitpy.common.net.rietveld import Rietveld
     32from webkitpy.thirdparty.mock import Mock
    3333
    3434
    35 class PostDiff(AbstractStep):
    36     @classmethod
    37     def options(cls):
    38         return [
    39             Options.description,
    40             Options.review,
    41             Options.request_commit,
    42             Options.open_bug,
    43         ]
    44 
    45     def run(self, state):
    46         diff = self.cached_lookup(state, "diff")
    47         diff_file = StringIO.StringIO(diff) # add_patch_to_bug expects a file-like object
    48         description = self._options.description or "Patch"
    49         self._tool.bugs.add_patch_to_bug(state["bug_id"], diff_file, description, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit)
    50         if self._options.open_bug:
    51             self._tool.user.open_url(self._tool.bugs.bug_url_for_bug_id(state["bug_id"]))
     35class RietveldTest(unittest.TestCase):
     36    def test_url_for_issue(self):
     37        rietveld = Rietveld(Mock())
     38        self.assertEqual(rietveld.url_for_issue(34223),
     39                         "https://codereview.appspot.com/34223")

  • trunk/WebKitTools/Scripts/webkitpy/thirdparty/__init__.py

    r56897 r57552  
    7070                  url_subpath="pep8-0.5.0/pep8.py")
    7171
     72
     73rietveld_dir = os.path.join(autoinstalled_dir, "rietveld")
     74installer = AutoInstaller(target_dir=rietveld_dir)
     75installer.install(url="http://webkit-rietveld.googlecode.com/svn/trunk/static/upload.py",
     76                  target_name="upload.py")
     77
     78
    7279# Since irclib and ircbot are two top-level packages, we need to import
    7380# them separately.  We group them into an irc package for better

  • trunk/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py

    r57451 r57552  
    7272        queue = TestQueue()
    7373        tool = MockTool()
     74        tool.executive = Mock()
    7475        queue.bind_to_tool(tool)
    7576

  • trunk/WebKitTools/Scripts/webkitpy/tool/commands/upload.py

    r57361 r57552  
    165165        steps.CheckStyle,
    166166        steps.ConfirmDiff,
     167        steps.PostCodeReview,
    167168        steps.ObsoletePatches,
    168169        steps.PostDiff,
    … …  
    209210        steps.EditChangeLog,
    210211        steps.ConfirmDiff,
     212        steps.PostCodeReview,
    211213        steps.ObsoletePatches,
    212214        steps.PostDiff,

  • trunk/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py

    r57137 r57552  
    5959        options.request_commit = False
    6060        options.review = True
     61        options.cc = None
    6162        expected_stderr = "Running check-webkit-style\nObsoleting 2 old patches on bug 42\nMOCK add_patch_to_bug: bug_id=42, description=MOCK description, mark_for_review=True, mark_for_commit_queue=False, mark_for_landing=False\n-- Begin comment --\nNone\n-- End comment --\nMOCK: user.open_url: http://example.com/42\n"
    6263        self.assert_execute_outputs(Post(), [42], options=options, expected_stderr=expected_stderr)
    … …  
    7879        options.request_commit = False
    7980        options.review = True
     81        options.cc = None
    8082        expected_stderr = "Running check-webkit-style\nObsoleting 2 old patches on bug 42\nMOCK add_patch_to_bug: bug_id=42, description=MOCK description, mark_for_review=True, mark_for_commit_queue=False, mark_for_landing=False\n-- Begin comment --\nNone\n-- End comment --\nMOCK: user.open_url: http://example.com/42\n"
    8183        self.assert_execute_outputs(Upload(), [42], options=options, expected_stderr=expected_stderr)

  • trunk/WebKitTools/Scripts/webkitpy/tool/main.py

    r56658 r57552  
    3737from webkitpy.common.net.bugzilla import Bugzilla
    3838from webkitpy.common.net.buildbot import BuildBot
     39from webkitpy.common.net.rietveld import Rietveld
    3940from webkitpy.common.net.irc.ircproxy import IRCProxy
    4041from webkitpy.common.system.executive import Executive
    … …  
    7172        self._checkout = None
    7273        self.status_server = StatusServer()
     74        self.codereview = Rietveld(self.executive)
    7375
    7476    def scm(self):
    … …  
    122124            self.scm().dryrun = True
    123125            self.bugs.dryrun = True
     126            self.codereview.dryrun = True
    124127        if options.status_host:
    125128            self.status_server.set_host(options.status_host)

  • trunk/WebKitTools/Scripts/webkitpy/tool/mocktool.py

    r57137 r57552  
    3434from webkitpy.common.checkout.scm import CommitMessage
    3535from webkitpy.common.net.bugzilla import Bug, Attachment
     36from webkitpy.common.net.rietveld import Rietveld
    3637from webkitpy.thirdparty.mock import Mock
    3738from webkitpy.common.system.deprecated_logging import log
    … …  
    4445    return dictionary
    4546
     47# Testing
    4648
    4749# FIXME: The ids should be 1, 2, 3 instead of crazy numbers.
    … …  
    489491
    490492
     493class MockExecute(Mock):
     494    def run_and_throw_if_fail(self, args, quiet=False):
     495        return "MOCK output of child process"
     496
     497
    491498class MockTool():
    492499
    … …  
    495502        self.bugs = MockBugzilla()
    496503        self.buildbot = MockBuildBot()
    497         self.executive = Mock()
     504        self.executive = MockExecute()
    498505        if log_executive:
    499506            self.executive.run_and_throw_if_fail = lambda args: log("MOCK run_and_throw_if_fail: %s" % args)
    … …  
    504511        self.status_server = MockStatusServer()
    505512        self.irc_password = "MOCK irc password"
     513        self.codereview = Rietveld(self.executive)
    506514
    507515    def scm(self):

  • trunk/WebKitTools/Scripts/webkitpy/tool/steps/__init__.py

    r56497 r57552  
    4545from webkitpy.tool.steps.obsoletepatches import ObsoletePatches
    4646from webkitpy.tool.steps.options import Options
     47from webkitpy.tool.steps.postcodereview import PostCodeReview
    4748from webkitpy.tool.steps.postdiff import PostDiff
    4849from webkitpy.tool.steps.postdiffforcommit import PostDiffForCommit

  • trunk/WebKitTools/Scripts/webkitpy/tool/steps/options.py

    r57424 r57552  
    4141    description = make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment (default: \"patch\")")
    4242    email = make_option("--email", action="store", type="string", dest="email", help="Email address to use in ChangeLogs.")
     43    fancy_review = make_option("--fancy-review", action="store_true", dest="fancy_review", default=False, help="(Experimental) Upload the patch to Rietveld code review tool.")
    4344    force_clean = make_option("--force-clean", action="store_true", dest="force_clean", default=False, help="Clean working directory before applying patches (removes local changes and commits)")
    4445    local_commit = make_option("--local-commit", action="store_true", dest="local_commit", default=False, help="Make a local commit for each applied patch")

  • trunk/WebKitTools/Scripts/webkitpy/tool/steps/postdiff.py

    r56497 r57552  
    4747        diff_file = StringIO.StringIO(diff) # add_patch_to_bug expects a file-like object
    4848        description = self._options.description or "Patch"
    49         self._tool.bugs.add_patch_to_bug(state["bug_id"], diff_file, description, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit)
     49        comment_text = None
     50        codereview_issue = state.get("codereview_issue")
     51        if codereview_issue:
     52            comment_text = "Feel free to provide comments at %s" % self._tool.codereview.url_for_issue(codereview_issue)
     53        self._tool.bugs.add_patch_to_bug(state["bug_id"], diff_file, description, comment_text=comment_text, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit)
    5054        if self._options.open_bug:
    5155            self._tool.user.open_url(self._tool.bugs.bug_url_for_bug_id(state["bug_id"]))
Note: See TracChangeset for help on using the changeset viewer.