Bug 26755 - webkit-patch's commit messages are less readable than commit-log-editor's
Summary: webkit-patch's commit messages are less readable than commit-log-editor's
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 32725
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-26 07:42 PDT by David Levin
Modified: 2011-07-11 12:32 PDT (History)
10 users (show)

See Also:


Attachments
Patch (6.30 KB, patch)
2009-12-03 23:53 PST, Adam Barth
no flags Details | Formatted Diff | Diff
Work in progress with unit test (9.03 KB, patch)
2009-12-04 00:40 PST, Adam Barth
no flags Details | Formatted Diff | Diff
Extract commit-log-editor's commit-message-generation code into a separate function (8.50 KB, patch)
2011-07-07 05:31 PDT, Adam Roben (:aroben)
ddkilzer: review+
Details | Formatted Diff | Diff
Add a --print-log option to commit-log-editor (2.84 KB, patch)
2011-07-07 05:34 PDT, Adam Roben (:aroben)
no flags Details | Formatted Diff | Diff
Add a --print-log option to commit-log-editor (2.71 KB, patch)
2011-07-07 08:33 PDT, Adam Roben (:aroben)
ddkilzer: review+
Details | Formatted Diff | Diff
Teach webkitpy's Checkout class to use commit-log-editor to create commit messages (6.83 KB, patch)
2011-07-07 08:41 PDT, Adam Roben (:aroben)
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-06-26 10:41:52 PDT
Bugzilla seems to have lost this description due to DB corruption last night.  The original description:

Compare http://trac.webkit.org/changeset/45257
to http://trac.webkit.org/changeset/45188

Two differences:
1. The lack of titles ("WebCore:", etc.)  to tell you what directory the
changelog is for.
2. The sections are sorted in a particular order.
Comment 1 Eric Seidel (no email) 2009-06-26 10:43:47 PDT
This is a known bug.  There is a FIXME about it in the source.  I certainly welcome patches to fix it.  Although once we add this to bugzilla-tool (or really some changelogs.py) then we should consider re-writing commit-log-editor in python, as this I believe is the only missing feature from our python modules that commit-log-editor needs.
Comment 2 Eric Seidel (no email) 2009-06-26 10:44:49 PDT
The source w/ FIXME:
http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/bugzilla-tool#L97
Comment 3 David Levin 2009-06-26 10:51:55 PDT
Yep,  I know it is a known bug (esp. since I flagged it in review :) ), but thought it worth logging a bug since the tool is being used to land patches that expose this.
Comment 4 Eric Seidel (no email) 2009-06-26 11:11:44 PDT
Totally!  Thanks for filing.
Comment 5 Mark Rowe (bdash) 2009-06-26 11:13:22 PDT
I don't think that we should be exactly matching the format of commit message in <http://trac.webkit.org/changeset/45188>.  In particular, we should avoid having useless info in the first line of the commit.  Hoisting the bug title up to that first line, above the blocks for each directory, would be a good idea.  I also think it's preferable to omit the redundant date / author lines from the commit message when the author is landing their own patches.
Comment 6 Eric Seidel (no email) 2009-08-11 23:23:33 PDT
Ideally what will happen is we'll convert commit-log-editor to a python module and re-use it from within bugzilla-tool.
Comment 7 David Kilzer (:ddkilzer) 2009-09-19 06:58:26 PDT
In Bug 29206 Comment #8, I wrote:

BTW bugzilla-tool still differs from commit-log-editor (see <http://trac.webkit.org/changeset/48503>):

- bzt does't strip spaces to the left of the commit log.
- bzt doesn't change date/name/email lines to subdirectory names.
- bzt puts the LayoutTest/ChangeLog entry first (not last).
- bzt is not putting a blank line between ChangeLog entries.
- bzt doesn't consolidate common lines from each ChangeLog entry to the top of the commit log.

Having said that, I don't like how commit-log-editor squishes the "Directory:" headings into the first line of text after it (see <http://trac.webkit.org/changeset/48492> for example).
Comment 8 Adam Barth 2009-12-03 23:53:06 PST
Created attachment 44296 [details]
Patch
Comment 9 WebKit Review Bot 2009-12-03 23:56:04 PST
style-queue ran check-webkit-style on attachment 44296 [details] without any errors.
Comment 10 Adam Barth 2009-12-04 00:39:32 PST
Comment on attachment 44296 [details]
Patch

In trying to write a unit test for this patch, we discovered that commit-log-editor is super fragile and has a number of error conditions that are very complex to recover from.  I no longer think this is the right approach.  We need a simpler way of creating commit messages that is testable.
Comment 11 Adam Barth 2009-12-04 00:40:22 PST
Created attachment 44297 [details]
Work in progress with unit test
Comment 12 Adam Roben (:aroben) 2011-05-26 09:59:59 PDT
Can we just invoke commit-log-editor?
Comment 13 Adam Barth 2011-05-26 10:12:21 PDT
> Can we just invoke commit-log-editor?

commit-log-editor is not designed in a way that it can be re-used outside of the SVN commit workflow.  One approach is to refactor commit-log-editor to work like a normal UNIX command and then use that core both in the SVN commit workflow and in the webkit-patch workflow.
Comment 14 Adam Roben (:aroben) 2011-07-01 07:36:25 PDT
A lot of the comments in this bug apply to patches with only a single ChangeLog. Retitling to include that case.
Comment 15 Adam Barth 2011-07-01 11:15:05 PDT
> Can we just invoke commit-log-editor?

By the way, we've got enough infrastructure now that we might be able to use this approach.  It might be worth investigating if folks like the format that commit-log-editor produces.
Comment 16 Adam Roben (:aroben) 2011-07-05 06:33:50 PDT
(In reply to comment #15)
> > Can we just invoke commit-log-editor?
> 
> By the way, we've got enough infrastructure now that we might be able to use this approach.  It might be worth investigating if folks like the format that commit-log-editor produces.

Can you elaborate a bit? I'd like to take a stab at fixing this.
Comment 17 Adam Barth 2011-07-05 14:46:07 PDT
> Can you elaborate a bit? I'd like to take a stab at fixing this.

aroben and I chatted a bit about this on IRC.  We talked about a couple different approaches.  Think the one that he seemed most interested in is to refactor commit-log-editor into two scripts: one the prints the commit message to stdout and another that interacts with SVN_EDITOR and svn.
Comment 18 Adam Roben (:aroben) 2011-07-05 15:01:53 PDT
(In reply to comment #17)
> > Can you elaborate a bit? I'd like to take a stab at fixing this.
> 
> aroben and I chatted a bit about this on IRC.  We talked about a couple different approaches.  Think the one that he seemed most interested in is to refactor commit-log-editor into two scripts: one the prints the commit message to stdout and another that interacts with SVN_EDITOR and svn.

Or perhaps one script with two modes.
Comment 19 Adam Roben (:aroben) 2011-07-07 05:31:00 PDT
Created attachment 99967 [details]
Extract commit-log-editor's commit-message-generation code into a separate function
Comment 20 Adam Roben (:aroben) 2011-07-07 05:34:57 PDT
Created attachment 99968 [details]
Add a --print-log option to commit-log-editor
Comment 21 Adam Roben (:aroben) 2011-07-07 08:33:55 PDT
Created attachment 99985 [details]
Add a --print-log option to commit-log-editor
Comment 22 Adam Roben (:aroben) 2011-07-07 08:41:53 PDT
Created attachment 99987 [details]
Teach webkitpy's Checkout class to use commit-log-editor to create commit messages
Comment 23 WebKit Review Bot 2011-07-07 08:45:05 PDT
Attachment 99987 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy..." exit_code: 1

Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:87:  trailing whitespace  [pep8/W291] [5]
Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:94:  trailing whitespace  [pep8/W291] [5]
Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:123:  expected 1 blank line, found 0  [pep8/E301] [5]
Total errors found: 3 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 Adam Roben (:aroben) 2011-07-07 08:54:51 PDT
Comment on attachment 99987 [details]
Teach webkitpy's Checkout class to use commit-log-editor to create commit messages

View in context: https://bugs.webkit.org/attachment.cgi?id=99987&action=review

>> Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:87
>> +Tools: 
> 
> trailing whitespace  [pep8/W291] [5]

This is required to match commit-log-editor's output. (We could change commit-log-editor not to output these extra spaces, but that should be done separately.)

>> Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:94
>> +LayoutTests: 
> 
> trailing whitespace  [pep8/W291] [5]

Ditto.

>> Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:123
>> +        def mock_script_path(script):
> 
> expected 1 blank line, found 0  [pep8/E301] [5]

I'll fix this locally.
Comment 25 David Kilzer (:ddkilzer) 2011-07-07 08:57:49 PDT
Comment on attachment 99967 [details]
Extract commit-log-editor's commit-message-generation code into a separate function

r=me
Comment 26 David Kilzer (:ddkilzer) 2011-07-07 09:06:12 PDT
Comment on attachment 99985 [details]
Add a --print-log option to commit-log-editor

View in context: https://bugs.webkit.org/attachment.cgi?id=99985&action=review

r=me, although consider fixing the exit issues.

> Tools/Scripts/commit-log-editor:50
>  sub usage

Consider renaming to printUsageAndExit() so it's more descriptive.

> Tools/Scripts/commit-log-editor:73
> +    exit !$getOptionsResult;

This exit doesn't do anything because usage() already exits.

> Tools/Scripts/commit-log-editor:81
> +        exit 1;

Ditto.

> Tools/Scripts/commit-log-editor:84
> +    exit;

"exit 0;" ?
Comment 27 Adam Roben (:aroben) 2011-07-07 09:14:54 PDT
Comment on attachment 99985 [details]
Add a --print-log option to commit-log-editor

View in context: https://bugs.webkit.org/attachment.cgi?id=99985&action=review

>> Tools/Scripts/commit-log-editor:50
>>  sub usage
> 
> Consider renaming to printUsageAndExit() so it's more descriptive.

Done.

>> Tools/Scripts/commit-log-editor:73
>> +    exit !$getOptionsResult;
> 
> This exit doesn't do anything because usage() already exits.

Good point! Removed.

>> Tools/Scripts/commit-log-editor:81
>> +        exit 1;
> 
> Ditto.

Removed.

>> Tools/Scripts/commit-log-editor:84
>> +    exit;
> 
> "exit 0;" ?

Sure, changed.
Comment 28 David Kilzer (:ddkilzer) 2011-07-07 09:31:40 PDT
Comment on attachment 99987 [details]
Teach webkitpy's Checkout class to use commit-log-editor to create commit messages

View in context: https://bugs.webkit.org/attachment.cgi?id=99987&action=review

r=me

> Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:124
> +            return os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', script))

There isn't a better way to get the path to the Tools directory (during a test)?
Comment 29 Adam Roben (:aroben) 2011-07-07 09:34:11 PDT
Comment on attachment 99987 [details]
Teach webkitpy's Checkout class to use commit-log-editor to create commit messages

View in context: https://bugs.webkit.org/attachment.cgi?id=99987&action=review

>> Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:124
>> +            return os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', script))
> 
> There isn't a better way to get the path to the Tools directory (during a test)?

Not that I know of. The SCM class knows how to find Tools/Scripts, but it bases it on the repository root, and we're operating in a fake repository here.
Comment 30 Adam Roben (:aroben) 2011-07-07 09:37:06 PDT
Committed r90564: <http://trac.webkit.org/changeset/90564>
Comment 31 Adam Barth 2011-07-07 10:41:16 PDT
Comment on attachment 99987 [details]
Teach webkitpy's Checkout class to use commit-log-editor to create commit messages

View in context: https://bugs.webkit.org/attachment.cgi?id=99987&action=review

> Tools/Scripts/webkitpy/common/checkout/checkout.py:123
> +        message_text = Executive().run_command([self._scm.script_path('commit-log-editor'), '--print-log'] + changelog_paths, return_stderr=False)

This isn't correct.  You need to get the executive from the tool so that it is mocked out correctly in unit tests.
Comment 32 Adam Barth 2011-07-07 10:45:15 PDT
@ddkilzer: This patch contains a couple architectural mistakes.  For example, grabbing the executive statically rather than using the dependency injection layer is a subtle but important violation of an invariant that we try to maintain across the entire codebase.

In the future, it might be better to give someone who works on webkitpy frequently a chance to look at a patch before landing it.
Comment 33 Eric Seidel (no email) 2011-07-07 10:47:34 PDT
Comment on attachment 99987 [details]
Teach webkitpy's Checkout class to use commit-log-editor to create commit messages

View in context: https://bugs.webkit.org/attachment.cgi?id=99987&action=review

> Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:85
> +    expected_commit_message = u"""Unreviewed build fix to un-break webkit-patch land.

Some of these intentionally contained unicode.  Do we still correctly support unicode commit messages?
Comment 34 Adam Roben (:aroben) 2011-07-07 10:49:08 PDT
Comment on attachment 99987 [details]
Teach webkitpy's Checkout class to use commit-log-editor to create commit messages

View in context: https://bugs.webkit.org/attachment.cgi?id=99987&action=review

>> Tools/Scripts/webkitpy/common/checkout/checkout.py:123
>> +        message_text = Executive().run_command([self._scm.script_path('commit-log-editor'), '--print-log'] + changelog_paths, return_stderr=False)
> 
> This isn't correct.  You need to get the executive from the tool so that it is mocked out correctly in unit tests.

This code is below the level of the tool. Using it here would be a layering violation, I believe. How do you suggest we make this work?
Comment 35 Adam Barth 2011-07-07 10:52:25 PDT
> This code is below the level of the tool. Using it here would be a layering violation, I believe. How do you suggest we make this work?

The tool has several layers.  For example, Checkout depends on SCM and SCM depends on Executive.  Checkout can either keep its own pointer to Executive or it can use the one that SCM has.  SCM initially gets that pointer from the tool when the tool is initialized.
Comment 36 Eric Seidel (no email) 2011-07-07 10:53:57 PDT
Looks like this broke test-webkitpy on the Gtk bot:
http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/24078/steps/webkitpy-test/logs/stdio

It suspect you're using a real executive when you mean to use MockExecutive.
Comment 37 Adam Roben (:aroben) 2011-07-07 10:55:20 PDT
Comment on attachment 99987 [details]
Teach webkitpy's Checkout class to use commit-log-editor to create commit messages

View in context: https://bugs.webkit.org/attachment.cgi?id=99987&action=review

>>> Tools/Scripts/webkitpy/common/checkout/checkout.py:123
>>> +        message_text = Executive().run_command([self._scm.script_path('commit-log-editor'), '--print-log'] + changelog_paths, return_stderr=False)
>> 
>> This isn't correct.  You need to get the executive from the tool so that it is mocked out correctly in unit tests.
> 
> This code is below the level of the tool. Using it here would be a layering violation, I believe. How do you suggest we make this work?

Note also that other code in this module uses the standalone run_command() function. I used Executive().run_command() because run_command() says to do that.

>> Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:85
>> +    expected_commit_message = u"""Unreviewed build fix to un-break webkit-patch land.
> 
> Some of these intentionally contained unicode.  Do we still correctly support unicode commit messages?

We do. I filed bug 64109 to prove it.
Comment 38 Adam Roben (:aroben) 2011-07-07 10:57:19 PDT
(In reply to comment #35)
> > This code is below the level of the tool. Using it here would be a layering violation, I believe. How do you suggest we make this work?
> 
> The tool has several layers.  For example, Checkout depends on SCM and SCM depends on Executive.  Checkout can either keep its own pointer to Executive or it can use the one that SCM has.  SCM initially gets that pointer from the tool when the tool is initialized.

OK, I'll fix this in a separate bug. Sorry for the mess.
Comment 39 Adam Roben (:aroben) 2011-07-07 11:00:22 PDT
(In reply to comment #36)
> Looks like this broke test-webkitpy on the Gtk bot:
> http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/24078/steps/webkitpy-test/logs/stdio
> 
> It suspect you're using a real executive when you mean to use MockExecutive.

MockExecutive seems to be part of the tool module. Is it really OK to use that in code inside the common module?
Comment 40 Adam Barth 2011-07-07 11:03:51 PDT
> Note also that other code in this module uses the standalone run_command() function. I used Executive().run_command() because run_command() says to do that.

That code is also wrong.  :)

The code that calls the standalone run_command() function all dates from the beginning of webkitpy, before we had any testing and before we re-designed for testing.  Eventually we'll clean up this code to follow the same invariants as the rest of webkitpy, but that's not been a super high priority.

That said, we don't want to spread this bad pattern to more locations, and when touching any code with the bad pattern, we usually update it to be more modern.  I certainly understand why you wrote the patch the way you did.  This sort of thing is supposed to be flagged in the review process.
Comment 41 Adam Barth 2011-07-07 11:06:01 PDT
> MockExecutive seems to be part of the tool module. Is it really OK to use that in code inside the common module?

MockExecutive should move into the common module.  If you don't want to do that in this patch, there's already MockExecutive2 in the common package, which is helpful for some kinds of tests.  (I'm not entirely sure why we have two kinds of MockExecutives.)
Comment 42 Adam Roben (:aroben) 2011-07-07 11:07:28 PDT
(In reply to comment #41)
> > MockExecutive seems to be part of the tool module. Is it really OK to use that in code inside the common module?
> 
> MockExecutive should move into the common module.  If you don't want to do that in this patch, there's already MockExecutive2 in the common package, which is helpful for some kinds of tests.  (I'm not entirely sure why we have two kinds of MockExecutives.)

Both MockExecutive and MockExecutive2 just return canned output. Is that really what we want?
Comment 43 Adam Barth 2011-07-07 11:11:03 PDT
> Both MockExecutive and MockExecutive2 just return canned output. Is that really what we want?

For most tests, yes.  If you specifically want to test integration with commit-log-editor, we can do that by explicitly passing a live Executive in the test.  Generally, we avoid unit tests that touch the real disk or spawn processes whenever possible.  We have a handful to make sure we continue to integrate correctly with external dependencies, but we don't need to test commit-log-editor exhaustively in test-webkitpy (we should just test that it works at all).
Comment 44 Adam Roben (:aroben) 2011-07-07 11:41:53 PDT
(In reply to comment #36)
> Looks like this broke test-webkitpy on the Gtk bot:
> http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/24078/steps/webkitpy-test/logs/stdio

Bug 64113 will fix the GTK bot regardless of whether we end up using a real Executive or not in the test.
Comment 45 Adam Roben (:aroben) 2011-07-07 11:55:53 PDT
(In reply to comment #43)
> > Both MockExecutive and MockExecutive2 just return canned output. Is that really what we want?
> 
> For most tests, yes.  If you specifically want to test integration with commit-log-editor, we can do that by explicitly passing a live Executive in the test.  Generally, we avoid unit tests that touch the real disk or spawn processes whenever possible.  We have a handful to make sure we continue to integrate correctly with external dependencies, but we don't need to test commit-log-editor exhaustively in test-webkitpy (we should just test that it works at all).

I think it makes sense to test the integration with commit-log-editor here. So here's my plan:

1) Make Checkout use SCM's Executive instance instead of conjuring one out of thin air
2) Make the mock SCM I create in the test use a real Executive instance

I'll file a new bug for this.
Comment 46 Adam Barth 2011-07-07 11:58:06 PDT
Sounds like a good plan.  Thanks.
Comment 47 Adam Roben (:aroben) 2011-07-07 12:00:59 PDT
(In reply to comment #45)
> (In reply to comment #43)
> > > Both MockExecutive and MockExecutive2 just return canned output. Is that really what we want?
> > 
> > For most tests, yes.  If you specifically want to test integration with commit-log-editor, we can do that by explicitly passing a live Executive in the test.  Generally, we avoid unit tests that touch the real disk or spawn processes whenever possible.  We have a handful to make sure we continue to integrate correctly with external dependencies, but we don't need to test commit-log-editor exhaustively in test-webkitpy (we should just test that it works at all).
> 
> I think it makes sense to test the integration with commit-log-editor here. So here's my plan:
> 
> 1) Make Checkout use SCM's Executive instance instead of conjuring one out of thin air
> 2) Make the mock SCM I create in the test use a real Executive instance
> 
> I'll file a new bug for this.

Bug 64115.
Comment 48 David Kilzer (:ddkilzer) 2011-07-07 15:34:02 PDT
(In reply to comment #32)
> @ddkilzer: This patch contains a couple architectural mistakes.  For example, grabbing the executive statically rather than using the dependency injection layer is a subtle but important violation of an invariant that we try to maintain across the entire codebase.

Where is this documented?  How is it enforced (other than patch reviews)?  Where would one go to learn about this?  Does one need to read the entire webkitpy (test) codebase to soak up this knowledge?

> In the future, it might be better to give someone who works on webkitpy frequently a chance to look at a patch before landing it.

Will do.  But I think it should be a goal of WebKit to make it easy to write tests (whether they're layout tests, webkitpy tests or webkitperl tests) so that more people can contribute without investing the better part of a day studying existing code for "subtle invariants" or requiring that one of only a handful of people review the patches.
Comment 49 Adam Barth 2011-07-07 15:56:28 PDT
> Where is this documented?

It's not document.

> How is it enforced (other than patch reviews)?

It's enforced by patch reviews.

> Where would one go to learn about this?

By writing patches for webkitpy and learning by interacting with reviewers.

> Does one need to read the entire webkitpy (test) codebase to soak up this knowledge?

How does one soak up knowledge about other parts of the codebase?  We generally ask that folks be active contributors to an area of code before reviewing patches in that area.

> Will do.  But I think it should be a goal of WebKit to make it easy to write tests (whether they're layout tests, webkitpy tests or webkitperl tests) so that more people can contribute without investing the better part of a day studying existing code for "subtle invariants" or requiring that one of only a handful of people review the patches.

That's what the review process is for.  The error is very easy to spot for folks who review and write code in this area often.  It's also easy to fix once spotted.  What's problematic is for folks to review patches in areas that they don't understand throughly.
Comment 50 David Kilzer (:ddkilzer) 2011-07-08 08:23:02 PDT
(In reply to comment #49)
> > Will do.  But I think it should be a goal of WebKit to make it easy to write tests (whether they're layout tests, webkitpy tests or webkitperl tests) so that more people can contribute without investing the better part of a day studying existing code for "subtle invariants" or requiring that one of only a handful of people review the patches.
> 
> That's what the review process is for.  The error is very easy to spot for folks who review and write code in this area often.  It's also easy to fix once spotted.  What's problematic is for folks to review patches in areas that they don't understand throughly.

I think it would also be beneficial if these "subtle invariants" could be made more explicit through either assertions (assuming Python has such a mechanism; maybe throwing exceptions?) or changes in the structure of the code to make it much harder to write incorrect code (which is likely harder in an interpreted scripting language than C++, but maybe still possible?).

Such changes would also decrease the amount of time it takes to explain these concepts when someone wants to contribute to the tools, since new tests would break immediately in a working copy if they violated the invariants.
Comment 51 Adam Barth 2011-07-08 08:36:09 PDT
I don't think that's possible in this case, but if it is, please let me know how!
Comment 52 Stephen White 2011-07-11 12:20:48 PDT
I notice that the ChangeLog entries on trac.webkit.org after r90564 seem to no longer be pretty-printed (ie., no indentation, no * -> bullets, etc).  Is it possible this change introduced a regression in that functionality?
Comment 53 Adam Roben (:aroben) 2011-07-11 12:23:19 PDT
(In reply to comment #52)
> I notice that the ChangeLog entries on trac.webkit.org after r90564 seem to no longer be pretty-printed (ie., no indentation, no * -> bullets, etc).  Is it possible this change introduced a regression in that functionality?

That is most definitely caused by this change. The lack of indentation is supposed to be a feature, not a bug, and is also what leads to * not being turned into a bullet. Maybe there's some better commit message format we can use, or maybe we can modify Trac to understand our commit messages better? I encourage you to file a new bug.
Comment 54 Stephen White 2011-07-11 12:32:24 PDT
(In reply to comment #53)
> (In reply to comment #52)
> > I notice that the ChangeLog entries on trac.webkit.org after r90564 seem to no longer be pretty-printed (ie., no indentation, no * -> bullets, etc).  Is it possible this change introduced a regression in that functionality?
> 
> That is most definitely caused by this change. The lack of indentation is supposed to be a feature, not a bug, and is also what leads to * not being turned into a bullet. Maybe there's some better commit message format we can use, or maybe we can modify Trac to understand our commit messages better? I encourage you to file a new bug.

Done; filed as https://bugs.webkit.org/show_bug.cgi?id=64307.