Add Feature Flag for Task Manager on Clank [chromium/src : main]
Erik Chen (Gerrit)
Erik Chen added 2 comments![]( "Open in Gerrit")
BASE_DECLARE_FEATURE(kTaskManagerClank);please move this elsewhere. please see file comment:
"""
// WARNING: do not add new entries here. If a feature is only used in one
// translation unit it should be inlined in that translation unit. If a feature
// is referenced in multiple places, it should be scoped to that module, e.g.
// //chrome/browser/<foo_module>/features.h
"""
"erik...@chromium.org",why am I added as a owner?
Related details
- Keigo Oka
- Michael Wojcicka
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wojcicka (Gerrit)
Michael Wojcicka added 2 comments![]( "Open in Gerrit")
BASE_DECLARE_FEATURE(kTaskManagerClank);please move this elsewhere. please see file comment:
"""
// WARNING: do not add new entries here. If a feature is only used in one
// translation unit it should be inlined in that translation unit. If a feature
// is referenced in multiple places, it should be scoped to that module, e.g.
// //chrome/browser/<foo_module>/features.h
"""
In that case, do you think that it makes more sense to make a new feature.h file in `chrome/browser/task_manager/` ?
One doesn't already exist, but I guess if there is a lot of work that is going on in this directory it might be worth while.
why am I added as a owner?
Are owners usually only people who are committing code under the flag?
Related details
- Erik Chen
- Keigo Oka
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keigo Oka (Gerrit)
Keigo Oka voted Code-Review+1![]( "Open in Gerrit")
- Erik Chen
- Michael Wojcicka
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Erik Chen (Gerrit)
Erik Chen added 2 comments![]( "Open in Gerrit")
BASE_DECLARE_FEATURE(kTaskManagerClank);Michael Wojcickaplease move this elsewhere. please see file comment:
"""
// WARNING: do not add new entries here. If a feature is only used in one
// translation unit it should be inlined in that translation unit. If a feature
// is referenced in multiple places, it should be scoped to that module, e.g.
// //chrome/browser/<foo_module>/features.h
"""
In that case, do you think that it makes more sense to make a new feature.h file in `chrome/browser/task_manager/` ?
One doesn't already exist, but I guess if there is a lot of work that is going on in this directory it might be worth while.
yep, that sounds good
Michael Wojcickawhy am I added as a owner?
Are owners usually only people who are committing code under the flag?
owners are people who are responsible for the feature -- e.g. let's say that in 3 years someone wants to know if it's okay to remove the flag, an owner would have the context necessary to answer that or other questions
Related details
- Michael Wojcicka
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Achuith Bhandarkar (Gerrit)
Achuith Bhandarkar added 3 comments![]( "Open in Gerrit")
"mw...@google.com"We should probably add the eng-velocity group as well
#if !BUILDFLAG(IS_ANDROID)should you just #else here?
Related details
- Michael Wojcicka
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wojcicka (Gerrit)
Michael Wojcicka added 4 comments![]( "Open in Gerrit")
Michael Wojcickawhy am I added as a owner?
Erik ChenAre owners usually only people who are committing code under the flag?
owners are people who are responsible for the feature -- e.g. let's say that in 3 years someone wants to know if it's okay to remove the flag, an owner would have the context necessary to answer that or other questions
Ah so sorry, wrong Eric! It should be lok...@google.com.
We should probably add the eng-velocity group as well
Done.
should you just #else here?
Done.
#if !BUILDFLAG(IS_ANDROID)Michael Wojcickaditto
Done
Related details
- Achuith Bhandarkar
- Erik Chen
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wojcicka (Gerrit)
Michael Wojcicka added 1 comment![]( "Open in Gerrit")
Michael Wojcickaplease move this elsewhere. please see file comment:
"""
// WARNING: do not add new entries here. If a feature is only used in one
// translation unit it should be inlined in that translation unit. If a feature
// is referenced in multiple places, it should be scoped to that module, e.g.
// //chrome/browser/<foo_module>/features.h
"""
Erik ChenIn that case, do you think that it makes more sense to make a new feature.h file in `chrome/browser/task_manager/` ?
One doesn't already exist, but I guess if there is a lot of work that is going on in this directory it might be worth while.
yep, that sounds good
- Erik Chen
- Keigo Oka
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Erik Chen (Gerrit)
Erik Chen voted Code-Review+1![]( "Open in Gerrit")
- Keigo Oka
- Michael Wojcicka
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wojcicka (Gerrit)
Michael Wojcicka voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
- Keigo Oka
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Add Feature Flag for Task Manager on Clank
- M chrome/browser/BUILD.gn
- M chrome/browser/about_flags.cc
- M chrome/browser/flag-metadata.json
- M chrome/browser/flag_descriptions.cc
- M chrome/browser/flag_descriptions.h
- A chrome/browser/task_manager/task_manager_features.cc
- A chrome/browser/task_manager/task_manager_features.h
- M tools/metrics/histograms/enums.xml
Code-Review: +1 by Erik Chen
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |