Changeset 56993 in webkit

Timestamp:

Apr 2, 2010 2:20:52 AM (14 years ago)

Author:

eric@webkit.org

Message:

2010-04-02 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

create-rollout doesn't fill out ChangeLog
​https://bugs.webkit.org/show_bug.cgi?id=37010

The contract between apply_reverse_diff and PrepareChangeLogForRevert
was unclear. I broke filling out the ChangeLog during rollout earlier
when I changed apply_reverse_diff to revert the ChangeLogs because
PrepareChangeLogForRevert thought that it was supposed to do that.
I've now taught PrepareChangeLogsForRevert the new contract.

It's unclear to me how to test this change because it's essentially an
integration issue that requires the file system. At some point we
should think about a testing strategy for integration. As the system
becomes larger, we're running into more of these issues.

• Scripts/webkitpy/common/checkout/api.py:
• Scripts/webkitpy/tool/steps/preparechangelogforrevert.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/common/checkout/api.py (modified) (1 diff)
• Scripts/webkitpy/tool/steps/preparechangelogforrevert.py (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-04-02  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        create-rollout doesn't fill out ChangeLog
+        https://bugs.webkit.org/show_bug.cgi?id=37010
+
+        The contract between apply_reverse_diff and PrepareChangeLogForRevert
+        was unclear.  I broke filling out the ChangeLog during rollout earlier
+        when I changed apply_reverse_diff to revert the ChangeLogs because
+        PrepareChangeLogForRevert thought that it was supposed to do that.
+        I've now taught PrepareChangeLogsForRevert the new contract.
+
+        It's unclear to me how to test this change because it's essentially an
+        integration issue that requires the file system.  At some point we
+        should think about a testing strategy for integration.  As the system
+        becomes larger, we're running into more of these issues.
+
+        * Scripts/webkitpy/common/checkout/api.py:
+        * Scripts/webkitpy/tool/steps/preparechangelogforrevert.py:
+
 2010-04-02  Adam Barth  <abarth@webkit.org>
 

trunk/WebKitTools/Scripts/webkitpy/common/checkout/api.py

@@ -128 +128 @@
         self._scm.apply_reverse_diff(revision)
 
-        # We don't want to remove the old ChangeLog entry.
+        # We revert the ChangeLogs because removing lines from a ChangeLog
+        # doesn't make sense.  ChangeLogs are append only.
         changelog_paths = self.modified_changelogs()
         if len(changelog_paths):

trunk/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelogforrevert.py

@@ -35 +35 @@
 class PrepareChangeLogForRevert(AbstractStep):
     def run(self, state):
-        # First, discard the ChangeLog changes from the rollout.
-        os.chdir(self._tool.scm().checkout_root)
-        changelog_paths = self._tool.checkout().modified_changelogs()
-        self._tool.scm().revert_files(changelog_paths)
-
-        # Second, make new ChangeLog entries for this rollout.
         # This could move to prepare-ChangeLog by adding a --revert= option.
         self._run_script("prepare-ChangeLog")
+        changelog_paths = self._tool.checkout().modified_changelogs()
         bug_url = self._tool.bugs.bug_url_for_bug_id(state["bug_id"]) if state["bug_id"] else None
         for changelog_path in changelog_paths: