Attempt to move ByteBufProxy into the main codebase so it can be used.#96
Attempt to move ByteBufProxy into the main codebase so it can be used.#96buko wants to merge 2 commits intolmdbjava:masterfrom
Conversation
|
Could somebody take a look at this? While this PR really fit for merging I thought it would be useful for an actual merge operation. At the least it would be good to provide some guidance here. @krisskross mentioned that tests were required, do we have more details on this? |
|
Sorry for the delay @buko. Im on vacation an trying to disconnect a little. Give some time and ill have a look. Thanks a lot for the contribution! |
|
Can you please do a fresh pull from master, as I have made many changes today including numerous dependency and plugin version updates, ensuring LmdbJava can compile on Java 11 (see Travis and AppVeyor builds), fixing the Build Number Maven Plugin so that it uses JGit and noticing an issue when using any version of Netty above 4.1.29. |
|
As discussed in #111 the Netty integration is currently not working, so I will close this PR. Please feel free to open a fresh PR when the tests pass with the latest Netty. |
Goal
Move ByteBufProxy into the main package so that it can be used by non-test clients. See #90 for more details.
Changes
Issues
mdbjava is fortunately a standard Maven project but I still ran into several issues when trying to make changes:
I've fixed this issue before in other projects many moons ago but forgot exactly how. I think it's possible to reconfigure the buildnumber plugin to use jgit instead of requiring the Git CLI. See https://github.com/alx3apps/jgit-buildnumber.
There are also alternatives to the buildnumber plugin that, for example, (eg https://github.com/release-engineering/buildmetadata-maven-plugin) that could be considered.
Was able to work around this issue by disabling the buildnumber plugin on the commandline/eclipse configuration.
This happens because the package org.lmdbjava in src/main/java and src/test/java define a 'package-info.java' file. Surprised the JDK allows this (it is a duplicate type) but Eclipse catches this and complains.
The build blows up on Java10. Got it working by using JDK 1.8.0_92
Ran into several issues with 'optional' dependencies. For now have made certain dependencies non-optional. Will need to investigate this further.
The checkstyle issues are very harsh and fail the build. Admittedly don't a lot of time to see how to reproduce the desired code formatting rules in Eclipse. For now was able to disable the checkstyle plugin, using -Dcheckstyle.skip=true.
Ran into PMD issues in an attempt to duplicate the tutorial test:
This was fixed by changing the tutorial test for Netty to use slightly different test values.
Note that the only files actually modified were:
pom.xml
ByteBufProxy.java
ByteBufProxyTest.java
TutorialTest.java
Conclusion
By working around all the issues was able to build the project with the command:
$ ~/data/applications/apache-maven-3.5.4/bin/mvn clean install -Dbuildnumber.phase=none -Dcheckstyle.skip=trueInstalled a SNAPSHOT locally that has the desired changes: lmdbjava-0.6.2-SNAPSHOT.
More work is needed to actually get this merged. Will try to coordinate further with lmdbjava team around this PR.