lobster: setup lobster client [chromium/src : main]
1 view
Skip to first unread message
Curtis McMullan (Gerrit)
Jun 26, 2024, 11:13:36 PM (6 days ago) Jun 26
to Darren Shen, chromium...@chromium.org, blundell+...@chromium.org
Attention needed from Darren Shen
Curtis McMullan added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Darren Shen
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:25:50 PM (6 days ago) Jun 26
to Curtis McMullan, chromium...@chromium.org, blundell+...@chromium.org
Attention needed from Curtis McMullan
Darren Shen voted and added 1 comment![]( "Open in Gerrit")
Votes added by Darren Shen
| Code-Review | +1 |
1 comment
File chrome/browser/ui/ash/lobster/lobster_client_impl.cc
Line 11, Patchset 8 (Latest):
bool LobsterClientImpl::IsFeatureAllowed() {Darren Shen . unresolved
nit: maybe add a TODO or NOTIMPLEMENTED_LOG_ONCE()
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:02:47 AM (yesterday) Jul 1
to Darren Shen, chromium...@chromium.org, blundell+...@chromium.org
Curtis McMullan added 1 comment![]( "Open in Gerrit")
File chrome/browser/ui/ash/lobster/lobster_client_impl.cc
nit: maybe add a TODO or NOTIMPLEMENTED_LOG_ONCE()
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:39:57 AM (yesterday) Jul 1
to Scott Violet, Darren Shen, chromium...@chromium.org, blundell+...@chromium.org
Attention needed from Scott Violet
Curtis McMullan added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 10 (Latest):
Curtis McMullan . resolved
PTAL thanks!
sky@ added for new folders with OWNERS files and browser main parts;
- //ash/public/cpp/lobster/OWNERS
- //chrome/browser/ui/ash/chrome_browser_main_parts_*
- //chrome/browser/ui/ash/lobster/OWNERS
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:11:20 AM (yesterday) Jul 1
to Curtis McMullan, Scott Violet, Darren Shen, chromium...@chromium.org, blundell+...@chromium.org
Attention needed from Curtis McMullan
Scott Violet voted and added 6 comments![]( "Open in Gerrit")
Votes added by Scott Violet
| Code-Review | +1 |
6 comments
Patchset-level comments
Scott Violet . resolved
Would be nice to have documentation of this, I'm assuming you're not doing this for specific reasons.
File ash/lobster/lobster_controller.h
Line 23, Patchset 10 (Latest):
LobsterClientFactory* client_factory_;Scott Violet . unresolved
raw_ptr?
File chrome/browser/ui/ash/lobster/lobster_client_factory_impl.h
Line 28, Patchset 10 (Latest):
ash::LobsterController* controller_;Scott Violet . unresolved
You're in the ash namespace, so no need for `ash::` here.
Line 28, Patchset 10 (Latest):
ash::LobsterController* controller_;Scott Violet . unresolved
If this can never change and is never null, use a raw_ref.
Line 10, Patchset 10 (Latest):
#include "ash/public/cpp/lobster/lobster_client.h"Scott Violet . unresolved
Please forward declare this here.
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:20 PM (2 hours ago) 9:20 PM
to Scott Violet, Darren Shen, chromium...@chromium.org, blundell+...@chromium.org
Curtis McMullan added 5 comments![]( "Open in Gerrit")
File ash/lobster/lobster_controller.h
Line 23, Patchset 10:
LobsterClientFactory* client_factory_;Scott Violet . resolved
Curtis McMullanraw_ptr?
Done
File chrome/browser/ui/ash/lobster/lobster_client_factory_impl.h
If this can never change and is never null, use a raw_ref.
Curtis McMullan
Done
Line 28, Patchset 10:
ash::LobsterController* controller_;Scott Violet . resolved
Curtis McMullanraw_ptr
Done
You're in the ash namespace, so no need for `ash::` here.
Curtis McMullan
I believe this is needed because we are in the global namespace (browser namespace) with LobsterClientFactoryImpl.
Please forward declare this here.
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Curtis McMullan (Gerrit)
10:45 PM (1 hour ago) 10:45 PM
to Scott Violet, Darren Shen, chromium...@chromium.org, blundell+...@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. |
Reply all
Reply to author
Forward
0 new messages