-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chrome_extensions: Add the profile name to the table #5213
Conversation
std::string profile_name; | ||
auto status = getChromeProfileName(profile_name, profile_path); | ||
if (!status.ok()) { | ||
LOG(WARNING) << status.getMessage(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I ask you to add a prefix for this log line? Smth like: "Getting chrome profile name failed: " ?
|
||
const auto& profile_obj = tree.get_child_optional(kProfilePreferenceKey); | ||
if (!profile_obj) { | ||
return Status(1, "The following profile is malformed: " + path.string()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: you could use a bit more self descriptive form of creating status with error:
return Status::failure(...);
If you like so :)
ok to test |
Hi @alessandrogario |
Hello @guliashvili ! I've updated the PR to target experimental! Thanks for looking into this patch! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@guliashvili has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@guliashvili merged this pull request in 507638d. |
This PR adds a new column to the
chrome_extensions
table namedprofile
, used to show the name of the Chrome profile that owns each extension.