Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2918563003:
Whitelisted Google Apps for public session (Closed)
|
Created:
3 years, 6 months ago by Ivan Šandrk Modified:
3 years, 6 months ago CC:
chromium-reviews, chromium-apps-reviews_chromium.org, oshima+watch_chromium.org, davemoore+watch_chromium.org, extensions-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionWhitelisted Google Apps for public session
Expanded whitelist according to spreadsheet linked in the bug:
https://docs.google.com/spreadsheets/d/1I3fUtUujjRZNBFd7suD1fzdsPJguZp4EESldHBo3IFw
BUG=712337
Review-Url: https://codereview.chromium.org/2918563003
Cr-Commit-Position: refs/heads/master@{#476622}
Committed: https://chromium.googlesource.com/chromium/src/+/af0e93c93117f537e337e65fe47358778f8ad63b
Patch Set 1 #Patch Set 2 : Nit - colon #Patch Set 3 : Moved whole list to Google Apps section now #Messages
Total messages: 24 (12 generated)
Description was changed from ========== Whitelisted Google Apps for public session Expanded whitelist according to spredsheet linked in the bug. BUG=712337 ========== to ========== Whitelisted Google Apps for public session Expanded whitelist according to spreadsheet linked in the bug: https://docs.google.com/spreadsheets/d/1I3fUtUujjRZNBFd7suD1fzdsPJguZp4EESldH... BUG=712337 ==========
isandrk@chromium.org changed reviewers: + atwilson@chromium.org
The CQ bit was checked by isandrk@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
On 2017/05/31 15:13:11, commit-bot: I haz the power wrote: > No L-G-T-M from a valid reviewer yet. > CQ run can only be started once the patch has received an L-G-T-M from a full > committer. > Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a > full super star committer. > Committers are members of the group "project-chromium-committers". > Note that this has nothing to do with OWNERS files. lgtm (testing something)
The CQ bit was checked by isandrk@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
isandrk@chromium.org changed reviewers: + pmarko@chromium.org
Hey Pavol, ptal!
On 2017/06/02 09:46:20, Ivan Šandrk wrote: > Hey Pavol, ptal! Hey Ivan :) Hm, at first I thought 3 were missing (Smart Card Connector, Chrome Remote Desktop, Camera), but it turned out these are in other blocks. What do the blocks mean - the reason why an app is whitelisted? Or - what is the difference between "Public Sessions in General" and "Google Apps", if "Public Sessions in General" also contains apps made by google? Also, it looks like the first 6 entries in the whitelist were started with alphabetical sorting in mind, which has been given up later - am I correct that we don't care about sorting?
On 2017/06/02 10:23:32, pmarko wrote: > On 2017/06/02 09:46:20, Ivan Šandrk wrote: > > Hey Pavol, ptal! > > Hey Ivan :) > > Hm, at first I thought 3 were missing (Smart Card Connector, Chrome Remote > Desktop, Camera), but it turned out these are in other blocks. After thinking about it a bit more, I've moved them all together in one section. > What do the blocks mean - the reason why an app is whitelisted? > Or - what is the difference between "Public Sessions in General" and "Google > Apps", if "Public Sessions in General" also contains apps made by google? Yeah exactly, the reason. Some are whitelisted because they're useful for testing, some are used at libraries/schools/retail, and some are needed by other customers. I guess the separation isn't that important, just that we whitelist what needs whitelisting. > Also, it looks like the first 6 entries in the whitelist were started with > alphabetical sorting in mind, which has been given up later - am I correct that > we don't care about sorting? Yeah, don't care :)
LGTM I'd leave it up to you to decide what to do with the sections, see below. On 2017/06/02 10:57:52, Ivan Šandrk wrote: > On 2017/06/02 10:23:32, pmarko wrote: > > On 2017/06/02 09:46:20, Ivan Šandrk wrote: > > > Hey Pavol, ptal! > > > > Hey Ivan :) > > > > Hm, at first I thought 3 were missing (Smart Card Connector, Chrome Remote > > Desktop, Camera), but it turned out these are in other blocks. > > After thinking about it a bit more, I've moved them all together in one section. I'd consider just moving 'Google Apps' to the first section, because I guess there are still google-made things up there (e.g. 'User Agent Switcher', 'ARC Runtime'). Or just leave it as it is, I really have no strong opinion about this. > > What do the blocks mean - the reason why an app is whitelisted? > > Or - what is the difference between "Public Sessions in General" and "Google > > Apps", if "Public Sessions in General" also contains apps made by google? > > Yeah exactly, the reason. Some are whitelisted because they're useful for > testing, some are used at libraries/schools/retail, and some are needed by other > customers. I guess the separation isn't that important, just that we whitelist > what needs whitelisting. Ack. > > > Also, it looks like the first 6 entries in the whitelist were started with > > alphabetical sorting in mind, which has been given up later - am I correct > that > > we don't care about sorting? > > Yeah, don't care :) Ack.
lgtm
The CQ bit was checked by isandrk@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks for the reviews!
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496405380805810,
"parent_rev": "9db7c8c8cffbef2695d74a148f3861914dc9e94c", "commit_rev":
"af0e93c93117f537e337e65fe47358778f8ad63b"}
Message was sent while issue was closed.
Description was changed from ========== Whitelisted Google Apps for public session Expanded whitelist according to spreadsheet linked in the bug: https://docs.google.com/spreadsheets/d/1I3fUtUujjRZNBFd7suD1fzdsPJguZp4EESldH... BUG=712337 ========== to ========== Whitelisted Google Apps for public session Expanded whitelist according to spreadsheet linked in the bug: https://docs.google.com/spreadsheets/d/1I3fUtUujjRZNBFd7suD1fzdsPJguZp4EESldH... BUG=712337 Review-Url: https://codereview.chromium.org/2918563003 Cr-Commit-Position: refs/heads/master@{#476622} Committed: https://chromium.googlesource.com/chromium/src/+/af0e93c93117f537e337e65fe473... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/af0e93c93117f537e337e65fe473... |
