Skip to content

Add Maven build.#4

Open
coder111111 wants to merge 1 commit intorayfowler:masterfrom
coder111111:maven
Open

Add Maven build.#4
coder111111 wants to merge 1 commit intorayfowler:masterfrom
coder111111:maven

Conversation

@coder111111
Copy link
Contributor

This time I left the files in place, for minimum change.

I added pom.xml. Deleted Apache math java files. Added 3rd party dependency on Apache math and packaging setup to include it in final jar. Modified gitignore to support IntelliJ Idea.

@matzon
Copy link

matzon commented Apr 13, 2020

I did the similar work in my fork - and I noticed the resource loading i quite a bit messy. While most of it goes through the Base::reader method, some other parts do their own thing.
I might look into sanitizing that, once the above patch lands.

fwiw, I also moved files to match the Apache Maven Standard Directory Layout. Makes the whole thing a lot cleaner, imo.

@matzon
Copy link

matzon commented Apr 13, 2020

Please note, the above patch introduces a version issue with Rotp.releaseId and the corresponding pom ${project.version}.
Ideally, that would be read in from a build-info like file, instead of hardcoded into the Rotp class.

@coder111111
Copy link
Contributor Author

coder111111 commented Apr 13, 2020 via email

@evanova
Copy link

evanova commented May 31, 2020

Would Gradle be a more satisfactory solution?

@coder111111
Copy link
Contributor Author

coder111111 commented May 31, 2020 via email

@evanova
Copy link

evanova commented Jun 2, 2020

Hi, Maybe it's my personal preference, but I prefer Maven. Two main reasons is that it's opinionated, simple and CONFIGURATION not code based. IMO if you need a programming language to specify how to do builds, you're screwing up and overcomplicated something. Building stuff should be simple easy. IMO it's easy to overcomplicate things with Gradle. It's harder to overcomplicate things with Maven. I've seen projects with 1000s lines of code in Gradle Groovy to get stuff built. It was nuts. I haven't seen projects that went so far overboard with Maven.

I have no opinion on the matter - what is convenient to the main maintainers of a project is always best. Here's my Gradle file for the curious (I am impressed how this project has zero dependencies btw)


plugins {
    id 'java'
    id 'application'
}

sourceCompatibility = "1.8"
targetCompatibility = "1.8"

group = 'rotp'
version = '1.13.b'

sourceSets {
    main {
        java {
            srcDirs = ['src']
            exclude 'src/images/**/*.*'
        }
        resources {
            srcDirs = ['src']
            exclude 'src/**/*.java'

        }
    }
}

mainClassName = "rotp.Rotp"

Use 'gradlew run' to start the app.

Copy link

@metteo metteo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest adding GitHub Action for mvn so the maintainer can easily verify if the change doesn't break anything.

In general, great work, wanted to propose something similar.

<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
Copy link

@metteo metteo Feb 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

groupId could be dropped since maven assumes that. On the other hand, version should be added to make builds stable.

</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

<modelVersion>4.0.0</modelVersion>
<groupId>com.rayfowler</groupId>
<artifactId>ROTP</artifactId>
<version>1.1</version>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.mojohaus.org/templating-maven-plugin/ could be used to transfer version into code. I suggest special class for that like ProjectInfo which contains constants from pom.

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.

5 participants