[webkit-reviews] review granted: [Bug 193916] Add data abstraction and validation for Ad Click Attribution : [Attachment 360432] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 29 10:49:06 PST 2019


Daniel Bates <dbates at webkit.org> has granted John Wilander
<wilander at apple.com>'s request for review:
Bug 193916: Add data abstraction and validation for Ad Click Attribution
https://bugs.webkit.org/show_bug.cgi?id=193916

Attachment 360432: Patch

https://bugs.webkit.org/attachment.cgi?id=360432&action=review




--- Comment #14 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 360432
  --> https://bugs.webkit.org/attachment.cgi?id=360432
Patch

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

> Source/WebCore/loader/AdClickAttribution.h:51
> +	       : id {id}

Thanks for switching to UIS, but if I had known you were just going to just
replace (/) with {/} and add a space before the { then I wouldn't have said
anything. There should be spaces inside the curly braces on both sides as well
just like how you wrote line 85 in the .cpp. Where is the description in the
ChangeLog? Where is the why or how comment on the 24-48 duration? Why do I
still see "Url" references in this patch? It's the little things like this that
give me pause when reviewing this patch (maybe for someone else they are just
noise?). I am done reviewing.


More information about the webkit-reviews mailing list