On Wed, Jul 22, 2009 at 1:17 PM, Ojan Vafai <o...@chromium.org> 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 < da...@apple.com <da...@apple.com>> > > 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. > > - 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: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
