Skip to content

Conversation

@ob-stripe
Copy link
Contributor

@ob-stripe ob-stripe commented Mar 29, 2019

r? @mickjermsurawong-stripe
cc @stripe/api-libraries

Use ErrorProne in builds.

Fixes #497.

There were only three issues remaining, two of which I think we can ignore:

  • ScheduledQueryRun.Error: ErrorProne complains that Error is a builtin class in java.lang and we should change the name. Given that ScheduledQueryRun.Error is a nested class and it's very unlikely any user would import it directly, I think it's safe to ignore this.
  • assertTrue(subscription.getTaxPercent().equals(new BigDecimal("0.3"))): ErrorProne warns that BigDecimal.equals compares the scale of the representation as well as the value (i.e. 1.0 != 1.00). This is actually desirable in our case, so this is a false positive.

Unfortunately, the last one is another missing @Override in autogenerated code, here:

Not sure how hard it would be to fix this. wdyt? Fixed, thanks Mick!

@ob-stripe
Copy link
Contributor Author

Oh oops, I just realized that ScheduledQueryRun.java is autogenerated too 🤦‍♂️

We'd need to have the generator add the @SuppressWarning annotation. I think this should be fairly simple (basically "if classname is Error then add the @SuppressWarning annotation).

@mickjermsurawong-stripe
Copy link
Contributor

@ob-stripe
do you think it's worth renaming the class? I can update the client-metadata and then we release it together with v9.0?

@mickjermsurawong-stripe
Copy link
Contributor

On @OverRide id. I can add that in the generator. Sorry didn't manage to get to.
But will do that before we release v9.0.

@ob-stripe ob-stripe force-pushed the ob-errorprone branch 2 times, most recently from 1735f69 to 0e2f06d Compare April 3, 2019 18:16
@ob-stripe ob-stripe mentioned this pull request Apr 5, 2019
9 tasks
@ob-stripe ob-stripe changed the title [WIP] Use ErrorProne in builds Use ErrorProne in builds Apr 8, 2019
@ob-stripe
Copy link
Contributor Author

Okay, this should be ready now. ptal @mickjermsurawong-stripe

@mickjermsurawong-stripe
Copy link
Contributor

LGTM
thank you @ob-stripe!

@ob-stripe ob-stripe merged commit 32d1cab into integration-v9 Apr 8, 2019
@ob-stripe ob-stripe deleted the ob-errorprone branch April 8, 2019 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants