Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2570043002:
[ios] Adds a buildflag for the ios_app_bundle_id_prefix. (Closed)
|
Created:
4 years ago by rohitrao (ping after 24h) Modified:
4 years ago Reviewers:
sdefresne CC:
chromium-reviews, pkl (ping after 24h if needed), sdefresne+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[ios] Adds a buildflag for the ios_app_bundle_id_prefix.
This will allow the prefix to be used as a string constant in code.
BUG=None
Committed: https://crrev.com/facf9c3631846243635ca6c98805bb443ad982ca
Cr-Commit-Position: refs/heads/master@{#439355}
Patch Set 1 #
Total comments: 5
Patch Set 2 : Moved to common/. #
Messages
Total messages: 21 (12 generated)
The CQ bit was checked by rohitrao@chromium.org 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...
rohitrao@chromium.org changed reviewers: + sdefresne@chromium.org
https://codereview.chromium.org/2570043002/diff/1/build/config/ios/BUILD.gn File build/config/ios/BUILD.gn (right): https://codereview.chromium.org/2570043002/diff/1/build/config/ios/BUILD.gn#n... build/config/ios/BUILD.gn:69: buildflag_header("ios_app_bundle_id_prefix_header") { Is this the right GN file for this header? https://codereview.chromium.org/2570043002/diff/1/ios/chrome/common/app_group... File ios/chrome/common/app_group/app_group_constants.mm (right): https://codereview.chromium.org/2570043002/diff/1/ios/chrome/common/app_group... ios/chrome/common/app_group/app_group_constants.mm:51: BUILDFLAG(IOS_APP_BUNDLE_ID_PREFIX), nil]; Is there a better way to concatenate these strings?
https://codereview.chromium.org/2570043002/diff/1/build/config/ios/BUILD.gn File build/config/ios/BUILD.gn (right): https://codereview.chromium.org/2570043002/diff/1/build/config/ios/BUILD.gn#n... build/config/ios/BUILD.gn:69: buildflag_header("ios_app_bundle_id_prefix_header") { On 2016/12/13 14:14:30, rohitrao wrote: > Is this the right GN file for this header? I think it would be better in ios/chrome/common (at some point I'd like to maybe move the ios_app_bundle_prefix variable there too). https://codereview.chromium.org/2570043002/diff/1/ios/chrome/common/app_group... File ios/chrome/common/app_group/app_group_constants.mm (right): https://codereview.chromium.org/2570043002/diff/1/ios/chrome/common/app_group... ios/chrome/common/app_group/app_group_constants.mm:51: BUILDFLAG(IOS_APP_BUNDLE_ID_PREFIX), nil]; On 2016/12/13 14:14:30, rohitrao wrote: > Is there a better way to concatenate these strings? Ah, BUILDFLAG(IOS_APP_BUNDLE_ID_PREFIX) expands to ("org.chromium") and we cannot use the concatenation from the preprocessor. That's lame. I don't think there is a better way that stringWithFormat here. Should only happens for non official builds though (since official sets KSApplicationGroup IIRC).
https://codereview.chromium.org/2570043002/diff/1/build/config/ios/BUILD.gn File build/config/ios/BUILD.gn (right): https://codereview.chromium.org/2570043002/diff/1/build/config/ios/BUILD.gn#n... build/config/ios/BUILD.gn:69: buildflag_header("ios_app_bundle_id_prefix_header") { On 2016/12/13 15:18:02, sdefresne wrote: > On 2016/12/13 14:14:30, rohitrao wrote: > > Is this the right GN file for this header? > > I think it would be better in ios/chrome/common (at some point I'd like to maybe > move the ios_app_bundle_prefix variable there too). So I'll leave the variable in ios_sdk.gni but I'll move the buildflag_header to common?
On 2016/12/13 15:20:48, rohitrao wrote: > https://codereview.chromium.org/2570043002/diff/1/build/config/ios/BUILD.gn > File build/config/ios/BUILD.gn (right): > > https://codereview.chromium.org/2570043002/diff/1/build/config/ios/BUILD.gn#n... > build/config/ios/BUILD.gn:69: > buildflag_header("ios_app_bundle_id_prefix_header") { > On 2016/12/13 15:18:02, sdefresne wrote: > > On 2016/12/13 14:14:30, rohitrao wrote: > > > Is this the right GN file for this header? > > > > I think it would be better in ios/chrome/common (at some point I'd like to > maybe > > move the ios_app_bundle_prefix variable there too). > > So I'll leave the variable in ios_sdk.gni but I'll move the buildflag_header to > common? Yes.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by rohitrao@chromium.org 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...
Updated to live in ios/chrome/common.
lgtm
The CQ bit was unchecked by rohitrao@chromium.org
The CQ bit was checked by rohitrao@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482005887615200,
"parent_rev": "581d84672380c75024a0f5b300252766cd0290cd", "commit_rev":
"1b5caffec88ef9720952bc81047baa9f72ecc13d"}
Message was sent while issue was closed.
Description was changed from ========== [ios] Adds a buildflag for the ios_app_bundle_id_prefix. This will allow the prefix to be used as a string constant in code. BUG=None ========== to ========== [ios] Adds a buildflag for the ios_app_bundle_id_prefix. This will allow the prefix to be used as a string constant in code. BUG=None Review-Url: https://codereview.chromium.org/2570043002 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [ios] Adds a buildflag for the ios_app_bundle_id_prefix. This will allow the prefix to be used as a string constant in code. BUG=None Review-Url: https://codereview.chromium.org/2570043002 ========== to ========== [ios] Adds a buildflag for the ios_app_bundle_id_prefix. This will allow the prefix to be used as a string constant in code. BUG=None Committed: https://crrev.com/facf9c3631846243635ca6c98805bb443ad982ca Cr-Commit-Position: refs/heads/master@{#439355} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/facf9c3631846243635ca6c98805bb443ad982ca Cr-Commit-Position: refs/heads/master@{#439355} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
