ios: Remove duplicate implementations of ReadStringSysctlByName [crashpad/crashpad : main]
1 view
Skip to first unread message
Justin Cohen (Gerrit)
May 30, 2024, 10:35:22 AMMay 30
to Mark Mentovai, Crashpad LUCI CQ, crashp...@chromium.org
Attention needed from Mark Mentovai
Justin Cohen voted and added 1 comment![]( "Open in Gerrit")
Votes added by Justin Cohen
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Mark Mentovai
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
May 30, 2024, 4:22:06 PMMay 30
to Justin Cohen, Crashpad LUCI CQ, crashp...@chromium.org
Attention needed from Justin Cohen
Mark Mentovai added 3 comments![]( "Open in Gerrit")
File test/ios/crash_type_xctest.mm
Line 40, Patchset 1 (Latest):
std::string build = crashpad::ReadStringSysctlByName("kern.osversion", false);Mark Mentovai . unresolved
Usually instead of this, we would do
```
namespace crashpad {
namespace {
…
```
at line 29.
File util/BUILD.gn
Line 402, Patchset 1 (Latest):
"mac/sysctl.cc",
"mac/sysctl.h",Mark Mentovai . unresolved
Put this in `crashpad_is_apple` above, and remove it from `crashpad_is_mac`.
File util/ios/ios_system_data_collector.mm
Line 81, Patchset 1 (Latest):
build_ = crashpad::ReadStringSysctlByName("kern.osversion", false);Mark Mentovai . unresolved
Can’t you see `crashpad` from within `crashpad::internal`?
Related details
Attention is currently required from:
- Justin Cohen
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Justin Cohen (Gerrit)
Jun 4, 2024, 12:08:10 PMJun 4
to Code Review Nudger, Mark Mentovai, Crashpad LUCI CQ, crashp...@chromium.org
Attention needed from Mark Mentovai
Justin Cohen added 3 comments![]( "Open in Gerrit")
File test/ios/crash_type_xctest.mm
Line 40, Patchset 1:
std::string build = crashpad::ReadStringSysctlByName("kern.osversion", false);Mark Mentovai . resolved
Usually instead of this, we would do
```
namespace crashpad {
namespace {
…
```at line 29.
Justin Cohen
Done
File util/BUILD.gn
Put this in `crashpad_is_apple` above, and remove it from `crashpad_is_mac`.
Justin Cohen
Done
File util/ios/ios_system_data_collector.mm
Line 81, Patchset 1:
build_ = crashpad::ReadStringSysctlByName("kern.osversion", false);Mark Mentovai . resolved
Can’t you see `crashpad` from within `crashpad::internal`?
Attention is currently required from:
- Mark Mentovai
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Jun 4, 2024, 1:21:19 PMJun 4
to Justin Cohen, Code Review Nudger, Crashpad LUCI CQ, crashp...@chromium.org
Attention needed from Justin Cohen
Mark Mentovai voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Justin Cohen
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Justin Cohen (Gerrit)
Jun 4, 2024, 1:24:59 PMJun 4
to Mark Mentovai, Code Review Nudger, Crashpad LUCI CQ, crashp...@chromium.org
Justin Cohen added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Justin Cohen . resolved
Are you OK with me landing while fuchsia is red?
Related details
Attention set is empty
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Jun 4, 2024, 1:29:09 PMJun 4
to Justin Cohen, Code Review Nudger, Crashpad LUCI CQ, crashp...@chromium.org
Attention needed from Justin Cohen
Mark Mentovai added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Mark Mentovai . resolved
Yes, you can land this non-Fuchsia-impacting change without those bots being green.
Related details
Attention is currently required from:
- Justin Cohen
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Justin Cohen (Gerrit)
Jun 4, 2024, 2:25:23 PMJun 4
to Mark Mentovai, Code Review Nudger, Crashpad LUCI CQ, crashp...@chromium.org
Justin Cohen submitted the change![]( "Open in Gerrit")
Change information
Commit message:
ios: Remove duplicate implementations of ReadStringSysctlByName
Bug: crashpad: 480
Change-Id: Ie37c557d2170f6d96968ec4922ec52bfc6ad8136
Reviewed-by: Mark Mentovai <ma...@chromium.org>
Files:
- M test/ios/crash_type_xctest.mm
- M util/BUILD.gn
- M util/ios/ios_system_data_collector.mm
Change size: S
Delta: 3 files changed, 10 insertions(+), 36 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Mark Mentovai
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages