GH-44016: [Java] Consider warnings as errors for Format Module #44017
Conversation
|
|
| @@ -15,6 +15,7 @@ | |||
| * limitations under the License. | |||
| */ | |||
|
|
|||
| @SuppressWarnings({ "requires-automatic", "requires-transitive-automatic" }) | |||
There was a problem hiding this comment.
@lidavidm is this acceptable?
There was a problem hiding this comment.
I thought we upgraded flatbuffers to avoid this?
There was a problem hiding this comment.
@danepitkin we did an update recently?
There was a problem hiding this comment.
We created a PR in flatbuffers, but it hasn't been reviewed yet. google/flatbuffers#8367
There was a problem hiding this comment.
Typical Google project. Well, that means this sorta defeats the purpose of modularization in the first place, but I suppose there's no alternative.
There was a problem hiding this comment.
Let's link to the Flatbuffers issue and clearly note that it's Google's problem here
There was a problem hiding this comment.
Sure, I will do that.
There was a problem hiding this comment.
Added a comment by linking https://github.com/apache/arrow/issues/44037
github-actions
bot
added
awaiting review
github-actions
bot
added
awaiting changes
| @@ -15,6 +15,7 @@ | |||
| * limitations under the License. | |||
| */ | |||
|
|
|||
| @SuppressWarnings({ "requires-automatic", "requires-transitive-automatic" }) | |||
There was a problem hiding this comment.
We created a PR in flatbuffers, but it hasn't been reviewed yet. google/flatbuffers#8367
github-actions
bot
added
awaiting change review
github-actions
bot
added
awaiting changes
Co-authored-by: David Li <[email protected]>
github-actions
bot
added
awaiting change review
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 22a4496. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 6 possible false positives for unstable benchmarks that are known to sometimes produce them. |
…pache#44017) ### Rationale for this change This PR configs the build such that warnings are considered errors in the Format module. And corresponding code changes have also been made. ### What changes are included in this PR? Adding flags to consider warnings as errors in javac and fixing the corresponding errors. ### Are these changes tested? Tested by existing test cases. ### Are there any user-facing changes? N/A * GitHub Issue: apache#44016 Lead-authored-by: Vibhatha Lakmal Abeykoon <[email protected]> Co-authored-by: Vibhatha Lakmal Abeykoon <[email protected]> Co-authored-by: David Li <[email protected]> Signed-off-by: Dane Pitkin <[email protected]>
…pache#44017) ### Rationale for this change This PR configs the build such that warnings are considered errors in the Format module. And corresponding code changes have also been made. ### What changes are included in this PR? Adding flags to consider warnings as errors in javac and fixing the corresponding errors. ### Are these changes tested? Tested by existing test cases. ### Are there any user-facing changes? N/A * GitHub Issue: apache#44016 Lead-authored-by: Vibhatha Lakmal Abeykoon <[email protected]> Co-authored-by: Vibhatha Lakmal Abeykoon <[email protected]> Co-authored-by: David Li <[email protected]> Signed-off-by: Dane Pitkin <[email protected]>
Rationale for this change
This PR configs the build such that warnings are considered errors in the Format module. And corresponding code changes have also been made.
What changes are included in this PR?
Adding flags to consider warnings as errors in javac and fixing the corresponding errors.
Are these changes tested?
Tested by existing test cases.
Are there any user-facing changes?
N/A