Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Morphed to Maven Project #1

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

lennartj
Copy link

Updates implemented

  • Split into 2 projects (API and JNA Impl), to separate the two and enable compiling only against the types within the API
  • Removed static JNA dependency from project. Added JNA as a normal Maven dependency.
  • Added Maven build files, with fleshed-out organisation and project information.
  • Added Intarsys License in the Maven-License-Plugin supported format. Placed license files in codestyle project.

@eheck-is
Copy link
Contributor

eheck-is commented Jul 5, 2022

Hi lennartj
Thank you for your contribution. The commits in this request, however, deal with very different topics, and while we'd like to accept some of them, we're not so sure about others. Could you please put your commits in separate pull requests, so we can decide on a one-by-one basis? Thank you.

@lennartj
Copy link
Author

lennartj commented Jul 5, 2022

Hello there,

While it is certainly doable to redo the PR by splitting it up into several individual PR:s,
it involves a bit of work. Hence, it would be good to know which parts of the PR you like and
which ones you dislike, implying I can focus on the simpler-to-accept bits.

Listing the changes here for convenience, along with my motivation for doing them. I think it
would be nice with a conversation regarding them to get to know your reasoning:

  1. Removed JNA Lib: Since the JNA library exists as a regular OpenSource dependency, I think
    it is simpler to let the build tool download it to include in the build. This also simplifies
    testing with other JNA versions.

  2. Removed Eclipse Project files: Most IDE project structures can be generated from the Maven POM,
    implying changes in the POMs will reflect in the IDEs in a simpler manner. This also means supporting
    other IDEs than Eclipse.

  3. Restructured into API and IMPL projects: This separates the API classes which a user must include
    in the compilation classpath, from the IMPL classes which a user should include in the runtime classpath.
    The original structure held all types in a single project, which means that developers could
    access both compile-time and runtime types from the same dependency inclusion.
    Using the structure in the PR, the typical pattern instead becomes (Maven):

  <dependency>
    <groupId>de.intarsys.opensource</groupId>
    <artifactId>nativec-api</artifactId>
    <version>${nativeCVersion}</version>
  </dependency>
  <dependency>
    <groupId>de.intarsys.opensource</groupId>
    <artifactId>nativec-jna-impl</artifactId>
    <version>${nativeCVersion}</version>
    <scope>runtime</scope>
  </dependency>

... and Gradle:

   api("de.intarsys.opensource:nativec-api:${nativeCVersion}")
   runtimeOnly("de.intarsys.opensource:nativec-jna-impl:${nativeCVersion}")
  1. Upgraded JNA to latest release: I realized that the JNA dependency you used was version
    4.2.x and that version 5.11.x was released. Hence, I upgraded the dependency and morphed the
    corresponding calls to the API of the newer JNA release.

  2. Added codestyle project holding the license: As always, I find it hard to ensure that
    licensing blurbs are consistent within the source code files. Hence, I engaged the Maven License
    Plugin from CodeHaus, and lifted the text found in the original source files into the MLP structure.
    I hope that it was done correctly, but you now have the opportunity to edit all licensing bits in
    a central location (i.e. the nativec-codestyle project's resource folder) to harmonize
    with your intentions. This actually goes on in 4 commits - 569e976, 67a943f, 23fa16c and 5cbbe14 -
    since it is involves a fair bit of moving directories.

  3. Changed to standard Markdown in README file: It is nice to be able to use GitHub-flavoured
    markdown for the formatting in github's GUI. I simply morphed the content of your textile file to a
    README.md file instead. In doing so, I also explained the structure (after the PR) of the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants