Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(23)

Issue 2858863002: Add macro to diff the groups (and their members) in ComputedStyleBase (Closed)

Created:
3 years, 7 months ago by nainar
Modified:
3 years, 7 months ago
Reviewers:
meade_UTC10, shend
CC:
blink-reviews, blink-reviews-style_chromium.org, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add macro to diff the groups (and their members) in ComputedStyleBase This patch adds the fieldwise_diff macro and then uses it to generate the diff functions on the groups that have been generated so far (StyleSurroundData) in ComputedStyleBase. Please note that it can only be used for memebers of those groups too that have already been generated. This is why the diffing for BorderData has been left to a later CL. Diff: https://gist.github.com/nainar/04f49165c4cb5ecb30371fbde1491ddf/revisions BUG=710938 Review-Url: https://codereview.chromium.org/2858863002 Cr-Commit-Position: refs/heads/master@{#469286} Committed: https://chromium.googlesource.com/chromium/src/+/997462ff044512808bc9f9ccc64b11148b625e91

Patch Set 1 #

Total comments: 6

Patch Set 2 : use results from diff functions generated in ComputedStyleBase.cpp in ComputedStyle.cpp #

Patch Set 3 : use results from diff functions generated in ComputedStyleBase.cpp in ComputedStyle.cpp #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -8 lines) Patch
M third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl View 1 2 2 chunks +20 lines, -1 line 0 comments Download
M third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.h.tmpl View 2 chunks +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/templates/fields/field.tmpl View 1 chunk +14 lines, -0 lines 1 comment Download
M third_party/WebKit/Source/core/style/ComputedStyle.cpp View 1 2 chunks +5 lines, -7 lines 0 comments Download

Messages

Total messages: 24 (17 generated)
nainar
shend@ PTAL? Thanks!
3 years, 7 months ago (2017-05-03 04:41:39 UTC) #4
shend
lgtm with nits https://codereview.chromium.org/2858863002/diff/1/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl File third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl (right): https://codereview.chromium.org/2858863002/diff/1/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl#newcode55 third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl:55: |selectattr("property_name", "in", ["bottom", "left", "margin-bottom", "margin-left", ...
3 years, 7 months ago (2017-05-03 05:18:12 UTC) #6
nainar
shend@ made the changes you suggested. meade@ for OWNERS https://codereview.chromium.org/2858863002/diff/1/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl File third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl (right): https://codereview.chromium.org/2858863002/diff/1/third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl#newcode55 third_party/WebKit/Source/build/scripts/templates/ComputedStyleBase.cpp.tmpl:55: ...
3 years, 7 months ago (2017-05-03 06:49:57 UTC) #12
meade_UTC10
lgtm It might be good to mention in the CL description how you chose naming ...
3 years, 7 months ago (2017-05-04 04:51:30 UTC) #17
nainar
On 2017/05/04 at 04:51:30, meade wrote: > lgtm > > It might be good to ...
3 years, 7 months ago (2017-05-04 04:59:00 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2858863002/40001
3 years, 7 months ago (2017-05-04 06:22:32 UTC) #21
commit-bot: I haz the power
3 years, 7 months ago (2017-05-04 06:25:55 UTC) #24
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/997462ff044512808bc9f9ccc64b...

Powered by Google App Engine
This is Rietveld 408576698