Here comes the bike shedding...but what about this: """
BUG= TEST= RELEASE_NOTES= """ I don't think any ALL CAPS TEXT at the top is going to make people put in better descriptions. If you want bug to be a url, it should use crbug.cominstead of the full string, but the convention so far has been a bar bug number which I think is fine. I don't really see why TEST is required...it's "none" much more often than BUG. I think simply populating it by default will get people to annotate it if they have anything to say. Otherwise they should just delete it. And I don't like the idea of needing to delete instructions on how to use the change descriptions every time I make a CL. I think what I'm proposing makes it obvious enough for newcomers while optimizing for people who submit a lot of CLs. J On Wed, Jul 22, 2009 at 1:43 PM, Ojan Vafai <[email protected]> wrote: > I'm OK with not having the function list. I can see the advantages of the > webkit approach as it means that comments in the code don't get outdated. > But, then it forces you to look at the changelog history of every line of > code. > Anyways, how about we prepopulate gcl/git-cl change with this text: > DETAILED_DESCRIPTION_HERE > > BUG=http://bugs.chromium.org/BUG_NUMBER_HERE > TEST=required > RELEASE_NOTES=optional > > > On Wed, Jul 22, 2009 at 1:28 PM, Jeremy Orlow <[email protected]> wrote: > >> On Wed, Jul 22, 2009 at 1:22 PM, Peter Kasting <[email protected]>wrote: >> >>> On Wed, Jul 22, 2009 at 1:17 PM, Ojan Vafai <[email protected]> wrote: >>> >>>> >>>>>> platform/graphics/chromium/FontPlatformDataLinux.cpp: >>>>>> >>>>> (WebCore::FontPlatformData::setHinting): >>>>>> (WebCore::FontPlatformData::setAntiAlias): >>>>>> (WebCore::FontPlatformData::setSubpixelGlyphs): >>>>>> (WebCore::FontPlatformData::setupPaint): Modified to do something >>>>>> super special. >>>>>> platform/graphics/chromium/FontPlatformDataLinux.h: >>>>>> >>>>> >>>>> Is it worth putting this into the change list? If so, maybe 'gcl >>>>> commit' can do it (since doing it any earlier risks it getting stale as >>>>> the >>>>> CL changes). >>>>> >>>> >>>> The benefit of this is that you can give more detailed comments inline. >>>> You see some WebKit commits do this (e.g. Darin Adler's) and it makes it >>>> much more clear from reading the change description what happened. So, if >>>> we're going to do it, it's not useful to do at commit time. It would be >>>> great if people got in the habit of writing detailed descriptions like >>>> this. >>>> >>> >>> I really, really hate this style. >>> >>> I have never encountered a case where a combination of comments-in-code >>> and good-change-description are not both sufficient, and far more readable >>> than the above style. >>> >>> Here's an example of a Darin Adler commit where this leads to a nice >>>> clear and detailed description. >>>> >>>> WebCore: >>>> >>>> 2009-07-15 Darin Adler < [email protected] <[email protected]>> >>>> >>>> Reviewed by John Sullivan. >>>> >>>> After double-clicking a word, using Shift-arrow to select behaves >>>> unpredictably >>>> >>>> https://bugs.webkit.org/show_bug.cgi?id=27177<https://bugs.webkit.org/show_bug.cgi?id=27177> >>>> rdar://problem/7034324 >>>> >>>> Test: editing/selection/extend-selection-after-double-click.html >>>> >>>> The bug was due to the m_lastChangeWasHorizontalExtension flag, which >>>> was not >>>> being cleared in many cases where it should have been. >>>> >>>> >>>> - editing/SelectionController.cpp: >>>> (WebCore::SelectionController::setSelection): Set >>>> m_lastChangeWasHorizontalExtension to false. This catches all sorts of >>>> cases >>>> that don't flow through the modify function. Before, the flag would >>>> reflect >>>> the last call to the modify function, which was not necessarily the last >>>> selection change. >>>> >>>> This kind of thing should be a comment in the code, and the fact that >>> WebKit does not generally comment code is much to their detriment. >>> >> >> Yes. For some reason, WebKit likes to replace comments in the code with >> comments in the ChangeLog which seems insane to me. If you need comments >> this detailed in the ChangeLog then something is wrong with the code IMHO. >> >> I think there are very few cases where you need down to the function level >> of detail. In most case, you should be putting comments in the code or >> splitting your CL into multiple that each address one specific issue. >> >>> >>>> - editing/selection/extend-selection-after-double-click.html: Copied >>>> from LayoutTests/editing/selection/word-granularity.html. Then turned it >>>> into a new test. >>>> >>>> This kind of thing, if really important, should be a comment in the >>> change description. >>> >>> PK >>> >> >> > --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
