You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "vibhatha (via GitHub)" <gi...@apache.org> on 2024/09/09 11:35:34 UTC
[PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
vibhatha opened a new pull request, #44017:
URL: https://github.com/apache/arrow/pull/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
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #44017:
URL: https://github.com/apache/arrow/pull/44017#discussion_r1751083832
##########
java/format/src/main/java/module-info.java:
##########
@@ -15,6 +15,7 @@
* limitations under the License.
*/
+@SuppressWarnings({ "requires-automatic", "requires-transitive-automatic" })
Review Comment:
Let's link to the Flatbuffers issue and clearly note that it's Google's problem here
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #44017:
URL: https://github.com/apache/arrow/pull/44017#discussion_r1751291365
##########
java/format/src/main/java/module-info.java:
##########
@@ -15,6 +15,8 @@
* limitations under the License.
*/
+// Remove warnings once criterion in https://github.com/apache/arrow/issues/44037 met.
Review Comment:
```suggestion
// TODO(https://github.com/apache/arrow/issues/44037): Google hasn't reviewed Flatbuffers fix
```
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #44017:
URL: https://github.com/apache/arrow/pull/44017#issuecomment-2342326736
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 22a44962bb3a641051cf449d9598477dd7dc820d.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/29963503176) has more details. It also includes information about 6 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #44017:
URL: https://github.com/apache/arrow/pull/44017#discussion_r1751083588
##########
java/format/src/main/java/module-info.java:
##########
@@ -15,6 +15,7 @@
* limitations under the License.
*/
+@SuppressWarnings({ "requires-automatic", "requires-transitive-automatic" })
Review 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.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #44017:
URL: https://github.com/apache/arrow/pull/44017#issuecomment-2337872790
:warning: GitHub issue #44016 **has been automatically assigned in GitHub** to PR creator.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #44017:
URL: https://github.com/apache/arrow/pull/44017#discussion_r1750196194
##########
java/format/src/main/java/module-info.java:
##########
@@ -15,6 +15,7 @@
* limitations under the License.
*/
+@SuppressWarnings({ "requires-automatic", "requires-transitive-automatic" })
Review Comment:
I thought we upgraded flatbuffers to avoid this?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #44017:
URL: https://github.com/apache/arrow/pull/44017#discussion_r1750393682
##########
java/format/src/main/java/module-info.java:
##########
@@ -15,6 +15,7 @@
* limitations under the License.
*/
+@SuppressWarnings({ "requires-automatic", "requires-transitive-automatic" })
Review Comment:
@danepitkin we did an update recently?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin merged PR #44017:
URL: https://github.com/apache/arrow/pull/44017
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #44017:
URL: https://github.com/apache/arrow/pull/44017#discussion_r1750093283
##########
java/format/src/main/java/module-info.java:
##########
@@ -15,6 +15,7 @@
* limitations under the License.
*/
+@SuppressWarnings({ "requires-automatic", "requires-transitive-automatic" })
Review Comment:
@lidavidm is this acceptable?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #44017:
URL: https://github.com/apache/arrow/pull/44017#discussion_r1751242452
##########
java/format/src/main/java/module-info.java:
##########
@@ -15,6 +15,7 @@
* limitations under the License.
*/
+@SuppressWarnings({ "requires-automatic", "requires-transitive-automatic" })
Review Comment:
Added a comment by linking https://github.com/apache/arrow/issues/44037
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "vibhatha (via GitHub)" <gi...@apache.org>.
vibhatha commented on code in PR #44017:
URL: https://github.com/apache/arrow/pull/44017#discussion_r1751220969
##########
java/format/src/main/java/module-info.java:
##########
@@ -15,6 +15,7 @@
* limitations under the License.
*/
+@SuppressWarnings({ "requires-automatic", "requires-transitive-automatic" })
Review Comment:
Sure, I will do that.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-44016: [Java] Consider warnings as errors for Format Module [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #44017:
URL: https://github.com/apache/arrow/pull/44017#discussion_r1750908106
##########
java/format/src/main/java/module-info.java:
##########
@@ -15,6 +15,7 @@
* limitations under the License.
*/
+@SuppressWarnings({ "requires-automatic", "requires-transitive-automatic" })
Review Comment:
We created a PR in flatbuffers, but it hasn't been reviewed yet. https://github.com/google/flatbuffers/pull/8367
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org