-
Notifications
You must be signed in to change notification settings - Fork 109
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
Remove is_clang_16 #3616
Remove is_clang_16 #3616
Conversation
865c976
to
2516b53
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3616 +/- ##
==========================================
- Coverage 58.74% 58.20% -0.54%
==========================================
Files 1781 1779 -2
Lines 85283 86040 +757
==========================================
- Hits 50099 50081 -18
- Misses 35184 35959 +775 ☔ View full report in Codecov by Sentry. |
586796d
to
e5fcd07
Compare
Corresponding internal change is here: go/lbreview/285820 Internal builders seem to pass the GN step as expected. PS5 fails on an unrelated change |
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.
Overall looks okay to put in. But you may want to double-check if all these places actually still do require this current_os != android
condition at all, especially in upstream code.
I know it will require a bit of trial and error back to back builds for Android, but it should be fairly quick.
What's the goal with this change? Why do we want to remove |
Change-Id: Id9bfd61837354f84b202bcd2d6b59c4d5401194a
e5fcd07
to
a92cd67
Compare
Removed !is_android in some places . remaining errors documented here: https://b.corp.google.com/issues/323184809#comment23 |
that variable was a placeholder when we moved to clang 16 a year ago, we have since updated to m114's clang 17 |
b/323184809 is_clang_16 is a temporary variable from a year ago which needs to be cleaned up as we have moved on to clang17. Replace `is_clang_16 `with `is_clang && current_os != "android" ` as android uses [clang version 14 ](go/auditmodularplatforms) (cherry picked from commit bc5acc2)
b/323184809
is_clang_16 is a temporary variable from a year ago which needs to be cleaned up as we have moved on to clang17.
Replace
is_clang_16
withis_clang && current_os != "android"
as android uses clang version 14