Skip to content
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

Changed compile SDK version to 33. #2693

Closed
wants to merge 5 commits into from
Closed

Conversation

OrangeAndGreen
Copy link
Contributor

Fixed a new error in DrawingBoundaryActivity.kt handling a nullable Location.

Summary

Changed from API 31 to 33 to support future work.

Safety Assurance

  • If the PR is high risk, "High Risk" label is set
  • [x ] I have confidence that this PR will not introduce a regression for the reasons below
  • Do we need to enhance manual QA test coverage ? If yes, "QA Note" label is set correctly

Safety story

A new compile error appeared in DrawingBoundaryActivity.kt involving safe usage of a nullable Location, so the code was changed slightly to handle the error.

Fixed a new error in DrawingBoundaryActivity.kt handling a nullable Location.
if (location != null && location.accuracy <= locationMinAccuracy) {
val addLocation = previousLocation == null ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious what was the compile error with earlier code ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previousLocation variable is a nullable Location but the distanceTo function won't accept null and generated a compile error. We actually null check previousLocation before entering the code block that calls distanceTo, but it's a class member so the compiler assumes it could be changed from somewhere else in the code. The fix was to simply retrieve the previousLocation into a local prevLocation variable, then null check and use that.

app/build.gradle Outdated
@@ -249,7 +249,7 @@ android {

defaultConfig {
minSdkVersion 21
targetSdkVersion 31
targetSdkVersion 33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OrangeAndGreen We can't change the targetSdkVersion without actually making the supporting Android 33 changes that Ahmad is working on. I think we should only be able to change the compileSdkVersion for incorporating the navigation UI component in code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@OrangeAndGreen OrangeAndGreen changed the title Changed compile and target SDK versions to 33. Changed compile SDK version to 33. Aug 23, 2023
shubham1g5
shubham1g5 previously approved these changes Aug 23, 2023
@OrangeAndGreen
Copy link
Contributor Author

@damagatchi Retest this please

1 similar comment
@shubham1g5
Copy link
Contributor

@damagatchi Retest this please

@shubham1g5
Copy link
Contributor

Actually Closing this in favour of #2683 as we need to get those changes in anyway.

@shubham1g5 shubham1g5 closed this Sep 13, 2023
@shubham1g5 shubham1g5 deleted the dv/compile_sdk_33 branch September 13, 2023 22:58
@avazirna avazirna added this to the 2.54 milestone Sep 14, 2023
@OrangeAndGreen OrangeAndGreen restored the dv/compile_sdk_33 branch May 23, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants