kbendick commented on pull request #1427: URL: https://github.com/apache/iceberg/pull/1427#issuecomment-687578777
> @kbendick , @rdblue , @aokolnychyi - This PR takes care of a good portion of the warnings currently in the code base. I tried to only take care of the ones I was sure were either actual bugs, or wouldn't actually change behavior. There are a few more classes of warning to clean up. I"ll try tackling those later. @RussellSpitzer I have a patch for `NarrowingCompoundAssignment` warnings, where I have to change a type in a class from `int` to `long` for correctness reasons. I can update that and then submit it tomorrow, as I imagine that's likely to see some discussion as it's less straight forward. I can submit it in the morning / afternoon tomorrow. I'm in agreement with @jzhuge that it would be great to start enforcing more of the ErrorProne warnings in gradle. As mentioned, if you don't know of any rules that we could start enforcing off of the top of your head, then I'd be more than happy to look into it. I know that I fixed a number of missing `@overrides` and as I didn't enforce it, more have slipped in. Though we might want to consider the length of time that the tests take to run vs how important erring out on specific warnings is (which is one reason why I'd prefer to see enforcement in gradle in probably a separate PR). ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
