Skip to content

Conversation

adrian-kong
Copy link
Contributor

Description

@swift-nav/devinfra

Adds @SuppressedWarning for unchecked generics and removes SBPStruct#parse return from generic to SBPStruct.

Changes the concurrent for loop removing to maybe prevent ConcurrentModificationException (?) - not sure if needed, also it doesn't look like it was being called anyways

API compatibility

Does this change introduce a API compatibility risk?

It should not, it only suppresses warnings and narrowing the scope of return type

JIRA Reference

https://swift-nav.atlassian.net/browse/DEVINFRA-479

Adds @SuppressedWarning for unchecked generics and removes SBPStruct#parse return from generic to SBPStruct.

Relates to DEVINFRA-479
@adrian-kong adrian-kong self-assigned this Apr 6, 2022
continue;
} catch (InterruptedException e) {
continue;
} catch (InterruptedException ignored) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think SonarCloud flagged this because you touched this line, if it makes sense we can just fix according to SonarCloud recommendation, if it doesn't make sense we can ignore the warning.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm yea, maybe we could log the error as it suggests but is also another thing which the library doesn't look like it uses a logger instead using System.out.println() everywhere

I don't think it should have to throw an InterruptedException and it follows logically like previously:
https://github.com/swift-nav/libsbp/runs/2882927558

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should ignore it (with // NOSONAR I think) and make a ticket to track switching to a logging facade

Copy link
Contributor

@silverjam silverjam left a comment

Choose a reason for hiding this comment

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

Ship it! 🎉

@adrian-kong adrian-kong marked this pull request as ready for review April 8, 2022 00:56
@adrian-kong adrian-kong requested a review from a team as a code owner April 8, 2022 00:56
@adrian-kong adrian-kong merged commit d470c03 into master Apr 8, 2022
@adrian-kong adrian-kong deleted the devinfra-479-fix-java-deprecation branch April 8, 2022 00:57
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