Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2187163002:
harfbuzz-ng: make use of the use_glib gn arg (Closed)
|
Created:
4 years, 4 months ago by Mostyn Bramley-Moore Modified:
4 years, 4 months ago Reviewers:
drott CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionharfbuzz-ng: make use of the use_glib gn arg
BUG=632297
Committed: https://crrev.com/923c83c5acd2bd5801c670dda6e87e1b0ec2778e
Cr-Commit-Position: refs/heads/master@{#408370}
Patch Set 1 #
Total comments: 2
Patch Set 2 : remove unnecessary use_glib #Messages
Total messages: 20 (12 generated)
The CQ bit was checked by mostynb@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
mostynb@opera.com changed reviewers: + drott@chromium.org
@drott: please take a look at this GN cleanup.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/07/27 at 22:21:12, mostynb wrote: > @drott: please take a look at this GN cleanup. Thanks for the CL, could you describe the context and intention a bit more? Do you have a bug for this change?
https://codereview.chromium.org/2187163002/diff/1/third_party/harfbuzz-ng/BUI... File third_party/harfbuzz-ng/BUILD.gn (right): https://codereview.chromium.org/2187163002/diff/1/third_party/harfbuzz-ng/BUI... third_party/harfbuzz-ng/BUILD.gn:55: !is_component_build && use_glib) { Why is use_glib added here? Because pango requires it? Perhaps moving it closer to use_pango could make that clearer.
Description was changed from ========== harfbuzz-ng: make use of the use_glib gn arg ========== to ========== harfbuzz-ng: make use of the use_glib gn arg BUG=632297 ==========
> Thanks for the CL, could you describe the context and intention a bit more? > Do you have a bug for this change? Added a bug reference. I am building a subset of the tree (content) for embedded linux, where glib, libgio etc are not available, so I have use_glib=false. Since //build/config/linux:glib is defined inside an if (use_glib) block, if any targets depend on this when I have use_glib=false then GN will error out. What was happening here was that we had a roughly equivalent expression for when to use glib (if linux and pango and so on), but which was not false and so caused GN to error out. IMO it's better to use these variables directly, to avoid such problems. It also makes refactoring easier, and these dependencies more obvious. https://codereview.chromium.org/2187163002/diff/1/third_party/harfbuzz-ng/BUI... File third_party/harfbuzz-ng/BUILD.gn (right): https://codereview.chromium.org/2187163002/diff/1/third_party/harfbuzz-ng/BUI... third_party/harfbuzz-ng/BUILD.gn:55: !is_component_build && use_glib) { On 2016/07/28 07:45:26, drott wrote: > Why is use_glib added here? Because pango requires it? Perhaps moving it closer > to use_pango could make that clearer. I added this here because this is dealing with a glib symbol, but this is probably not needed- I will remove it.
The CQ bit was checked by mostynb@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks for the background, LGTM.
The CQ bit was unchecked by mostynb@opera.com
The CQ bit was checked by mostynb@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== harfbuzz-ng: make use of the use_glib gn arg BUG=632297 ========== to ========== harfbuzz-ng: make use of the use_glib gn arg BUG=632297 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== harfbuzz-ng: make use of the use_glib gn arg BUG=632297 ========== to ========== harfbuzz-ng: make use of the use_glib gn arg BUG=632297 Committed: https://crrev.com/923c83c5acd2bd5801c670dda6e87e1b0ec2778e Cr-Commit-Position: refs/heads/master@{#408370} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/923c83c5acd2bd5801c670dda6e87e1b0ec2778e Cr-Commit-Position: refs/heads/master@{#408370} |
