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

Remove desugaring #62

Merged
merged 9 commits into from
May 20, 2024
Merged

Remove desugaring #62

merged 9 commits into from
May 20, 2024

Conversation

smihalic
Copy link
Contributor

@smihalic smihalic commented Apr 15, 2024

📝 Changes

There was an issue when updating AGP version from 8.0 to 8.3 where a project that uses sentinel would need to have desugaring enabled. Removing this also removes the need to add desugaring in other projects.

@smihalic smihalic requested review from antunflas, KCeh and AsimRibo April 15, 2024 15:24
@smihalic smihalic marked this pull request as ready for review April 15, 2024 15:24
@KCeh
Copy link
Collaborator

KCeh commented Apr 16, 2024

Correct, upgrading AGP to 8.3.0 will result in clients of library needing to use desugaring (https://issuetracker.google.com/issues/329346764)
So I agree it is probably better to remove it. But then we also need to change the code that is affected by this change.

@KCeh KCeh changed the base branch from master to develop April 16, 2024 06:23
@AsimRibo
Copy link
Contributor

@KCeh since removal of desugaring will take a lot more work, should we close this PR?

@KCeh
Copy link
Collaborator

KCeh commented Apr 22, 2024

@KCeh since removal of desugaring will take a lot more work, should we close this PR?

You are correct, it will take a lot more work. But let's keep it as a starting point. I will switch it to draft

@KCeh KCeh marked this pull request as draft April 22, 2024 12:26
Copy link

sonarqubecloud bot commented May 7, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@KCeh KCeh marked this pull request as ready for review May 12, 2024 13:34
@KCeh
Copy link
Collaborator

KCeh commented May 12, 2024

I have removed desugaring from Sentinel.
Sentinel's version of ChronoUnit is introduced to avoid the need for DB refactoring. Other than that in place where Java 8 Time API is needed checks and Require annotations are added. Therefore, certificate tools is no longer supported on devices API < 26

Copy link
Contributor

@AsimRibo AsimRibo left a comment

Choose a reason for hiding this comment

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

Looks good. Works good.

@KCeh KCeh merged commit 90c86c3 into develop May 20, 2024
6 checks passed
@KCeh KCeh deleted the remove-desugaring branch August 20, 2024 19:46
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