[Chromium] Linking error due to wrong guard in WebIDBTransaction.h
Created attachment 69034 [details] Patch
Comment on attachment 69034 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69034&action=review r=me > WebKit/chromium/public/WebIDBTransaction.h:65 > + // FIXME: this is never called in the renderer. Find a cleaner solution. Comments that apply to the whole function really seem like they should go right above the function not in them. Is there other precedent for something like this? If not, can you please stop doing this? Also, try not to make the comments Chromium architecture specific. Maybe just say that this is only meant to be called from within WebCore?
+fishd cause this is a webkit patch
(In reply to comment #2) > (From update of attachment 69034 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=69034&action=review > > r=me > > > WebKit/chromium/public/WebIDBTransaction.h:65 > > + // FIXME: this is never called in the renderer. Find a cleaner solution. > > Comments that apply to the whole function really seem like they should go right above the function not in them. Is there other precedent for something like this? If not, can you please stop doing this? > OK ok :) > Also, try not to make the comments Chromium architecture specific. Maybe just say that this is only meant to be called from within WebCore? Erm, this bug has the [Chromium] label and the patch touches only a file in /WebKit/chromium so how can making a chromium-specific comment possibly be wrong?
(In reply to comment #4) > (In reply to comment #2) > > (From update of attachment 69034 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=69034&action=review > > > > r=me > > > > > WebKit/chromium/public/WebIDBTransaction.h:65 > > > + // FIXME: this is never called in the renderer. Find a cleaner solution. > > > > Comments that apply to the whole function really seem like they should go right above the function not in them. Is there other precedent for something like this? If not, can you please stop doing this? > > > > OK ok :) > > > Also, try not to make the comments Chromium architecture specific. Maybe just say that this is only meant to be called from within WebCore? > > Erm, this bug has the [Chromium] label and the patch touches only a file in /WebKit/chromium so how can making a chromium-specific comment possibly be wrong? WebKit code should not depend on Chromium code. Even if this code is designed to be consumed by Chromium. Plus Darin said so. :-)
Committed r68504: <http://trac.webkit.org/changeset/68504>
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #2) > > > (From update of attachment 69034 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=69034&action=review > > > > > > r=me > > > > > > > WebKit/chromium/public/WebIDBTransaction.h:65 > > > > + // FIXME: this is never called in the renderer. Find a cleaner solution. > > > > > > Comments that apply to the whole function really seem like they should go right above the function not in them. Is there other precedent for something like this? If not, can you please stop doing this? > > > > > > > OK ok :) > > > > > Also, try not to make the comments Chromium architecture specific. Maybe just say that this is only meant to be called from within WebCore? > > > > Erm, this bug has the [Chromium] label and the patch touches only a file in /WebKit/chromium so how can making a chromium-specific comment possibly be wrong? > > WebKit code should not depend on Chromium code. Even if this code is designed to be consumed by Chromium. > Yes, that comment did depend on Chromium code...fair enough, changed.
(In reply to comment #8) > (In reply to comment #4) > > (In reply to comment #2) > > > (From update of attachment 69034 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=69034&action=review > > > > > > r=me > > > > > > > WebKit/chromium/public/WebIDBTransaction.h:65 > > > > + // FIXME: this is never called in the renderer. Find a cleaner solution. > > > > > > Comments that apply to the whole function really seem like they should go right above the function not in them. Is there other precedent for something like this? If not, can you please stop doing this? > > > > > > > OK ok :) > > > > > Also, try not to make the comments Chromium architecture specific. Maybe just say that this is only meant to be called from within WebCore? > > > > Erm, this bug has the [Chromium] label and the patch touches only a file in /WebKit/chromium so how can making a chromium-specific comment possibly be wrong? > > WebKit code should not depend on Chromium code. Even if this code is designed to be consumed by Chromium. > > Plus Darin said so. :-) Ugh...stupid bugzilla...sorry.