lobster: add feature flag [chromium/src : main]
1 view
Skip to first unread message
Darren Shen (Gerrit)
Jun 26, 2024, 11:12:42 PM (6 days ago) Jun 26
to Curtis McMullan, chromium...@chromium.org
Attention needed from Curtis McMullan
Darren Shen voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Curtis McMullan
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Darren Shen (Gerrit)
Jun 26, 2024, 11:14:04 PM (6 days ago) Jun 26
to Curtis McMullan, chromium...@chromium.org
Attention needed from Curtis McMullan
Darren Shen added 1 comment![]( "Open in Gerrit")
File ash/lobster/lobster_controller.h
Line 12, Patchset 2 (Latest):
static bool SecretKeyMatches();Darren Shen . unresolved
nit: maybe just `FeatureKeyMatches` is fine
Related details
Attention is currently required from:
- Curtis McMullan
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Curtis McMullan (Gerrit)
Jul 1, 2024, 12:01:53 AM (yesterday) Jul 1
to Darren Shen, chromium...@chromium.org
Curtis McMullan added 1 comment![]( "Open in Gerrit")
File ash/lobster/lobster_controller.h
nit: maybe just `FeatureKeyMatches` is fine
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Curtis McMullan (Gerrit)
Jul 1, 2024, 1:25:29 AM (yesterday) Jul 1
to Scott Violet, Darren Shen, chromium...@chromium.org
Attention needed from Scott Violet
Curtis McMullan added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 3 (Latest):
Curtis McMullan . resolved
PTAL thanks!
sky@ added for approval of new top level folder in //ash, thanks!
Related details
Attention is currently required from:
- Scott Violet
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Scott Violet (Gerrit)
Jul 1, 2024, 9:14:24 AM (yesterday) Jul 1
to Curtis McMullan, Scott Violet, Darren Shen, chromium...@chromium.org
Attention needed from Curtis McMullan
Scott Violet voted and added 2 comments![]( "Open in Gerrit")
Votes added by Scott Violet
| Code-Review | +1 |
2 comments
File ash/lobster/lobster_controller.h
Line 12, Patchset 3 (Latest):
static bool FeatureKeyMatches();Scott Violet . unresolved
Based on the name, it wasn't readily obvious what this does. Consider naming something like `IsEnabled()`.
File ash/lobster/lobster_controller.cc
Line 28, Patchset 3 (Latest):
const std::string provided_key_hash = base::SHA1HashString(
base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
switches::kLobsterFeatureKey));
return provided_key_hash == kLobsterKey;Scott Violet . unresolved
This seems non-trivial. I would be inclined to cache the value. That is, use a static boolean.
Related details
Attention is currently required from:
- Curtis McMullan
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Curtis McMullan (Gerrit)
9:10 PM (2 hours ago) 9:10 PM
to Scott Violet, Darren Shen, chromium...@chromium.org
Curtis McMullan added 2 comments![]( "Open in Gerrit")
File ash/lobster/lobster_controller.h
Based on the name, it wasn't readily obvious what this does. Consider naming something like `IsEnabled()`.
Curtis McMullan
Done
File ash/lobster/lobster_controller.cc
Line 28, Patchset 3:
const std::string provided_key_hash = base::SHA1HashString(
base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
switches::kLobsterFeatureKey));
return provided_key_hash == kLobsterKey;Scott Violet . resolved
This seems non-trivial. I would be inclined to cache the value. That is, use a static boolean.
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Curtis McMullan (Gerrit)
9:20 PM (2 hours ago) 9:20 PM
to Scott Violet, Darren Shen, chromium...@chromium.org
Curtis McMullan voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
10:04 PM (1 hour ago) 10:04 PM
to Curtis McMullan, Scott Violet, Darren Shen, chromium...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: ash/lobster/lobster_controller.cc
Insertions: 6, Deletions: 5.
The diff is too large to show. Please review the diff.
```
```
The name of the file: ash/lobster/lobster_controller.h
Insertions: 1, Deletions: 1.
The diff is too large to show. Please review the diff.
```
Change information
Commit message:
lobster: add feature flag
BUG=b:348280305
Change-Id: I7ce3135f0b10710783298604b8ebe71ffd7e0149
Reviewed-by: Darren Shen <sh...@chromium.org>
Commit-Queue: Curtis McMullan <curtism...@chromium.org>
Reviewed-by: Scott Violet <s...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1322529}
Files:
- M ash/BUILD.gn
- M ash/constants/ash_features.cc
- M ash/constants/ash_features.h
- M ash/constants/ash_switches.cc
- M ash/constants/ash_switches.h
- A ash/lobster/OWNERS
- A ash/lobster/lobster_controller.cc
- A ash/lobster/lobster_controller.h
Change size: M
Delta: 8 files changed, 71 insertions(+), 0 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Darren Shen, +1 by Scott Violet
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages