I created issues for 1,2 and 4. For #3 (Intellij hints/warnings), I'm not sure there is a good way to enforce this, so a one time cleanup could be helpful, but fear it going stale.
Thanks, Micah On Wed, Feb 27, 2019 at 1:16 PM Bryan Cutler <cutl...@gmail.com> wrote: > These all sound good to me Micah, thanks for taking this on! Regarding > the javadoc codestyle in (2), I believe it was disabled because there were > just too many issues of missing docs at the time. Any documentation > additions are definitely welcome and hopefully we can eventually enable the > check :) > > Thanks, > Bryan > > On Tue, Feb 26, 2019 at 10:17 PM Micah Kornfield <emkornfi...@gmail.com> > wrote: > >> SGTM, I probably start filing JIRAs on Friday if no objections arise. >> >> On Tue, Feb 26, 2019 at 12:25 AM Praveen Kumar <prav...@dremio.com> >> wrote: >> >> > Hi Micah, >> > >> > Thanks for bringing this up. >> > >> > I am +1 for all of them. Especially the lack of documentation in the the >> > classes is a little hard - i found myself checking documentation of >> > corresponding C++ classes to understand format initially. >> > >> > Lets wait a couple of days for more feedback and create actionable JIRAs >> > for the same? >> > >> > Thx. >> > >> > On Tue, Feb 26, 2019 at 11:34 AM Micah Kornfield <emkornfi...@gmail.com >> > >> > wrote: >> > >> >> Hi Java Arrow-Developers, >> >> I've been looking more into the java code base and I was wondering if >> >> people think any of the following might be worthwhile (or are strictly >> >> against them). My java infrastructure knowledge is a little stale, so >> if >> >> a >> >> suggestion I make is absolutely ridiculous I apologize. >> >> >> >> 1. Upgrade to JUnit 5 (the main advantage I know if is it has much >> >> cleaner >> >> syntax for asserts with Java8) [1] >> >> 2. Try to get a state where we can enforce with Checkstyle that at >> least >> >> every class has a javadoc comment? >> >> 3. Try to get the code to state where it shows no warnings in >> IntelliJ. >> >> At least in the Vector classes there seem to at least be warning about >> >> possibly making some methods/classes have more restricted visibility, I >> >> don't know if this due to insufficient test coverage or if they are >> >> legitimate. If it is the former, is there someplace we can pull in >> more >> >> tests to exercise the code? >> >> 4. Setup static type checking assuming NonNull values and annotate >> where >> >> values can be null (based on limited research [2][3][4] the checker >> >> framework might have the right set of annotations to use >> >> >> >> Thanks, >> >> Micah >> >> >> >> >> >> >> >> [1] >> >> >> >> >> https://stackoverflow.com/questions/40268446/junit-5-how-to-assert-an-exception-is-thrown >> >> [2] https://github.com/google/guava/issues/3031 >> >> [3] https://github.com/google/guava/issues/2960 >> >> [4] >> >> >> >> >> https://stackoverflow.com/questions/4963300/which-notnull-java-annotation-should-i-use >> >> >> > >> >
