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

Add a MakeFile #1513

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

Add a MakeFile #1513

wants to merge 3 commits into from

Conversation

Chopper1337
Copy link

@Chopper1337 Chopper1337 commented Mar 13, 2024

This pull request...

  • Fixes a bug
  • Introduces a new feature
  • Improves an existing feature
  • Boosts code quality or performance

Description

Add a Makefile for building,testing or cleaning.

Purpose

Allows developers to simply run make build or make test to build or test the application. Also allows for make clean to clean the ./target directory.

Relevant Issue(s)

N/A

@jagrosh
Copy link
Owner

jagrosh commented Mar 13, 2024

I have nothing against makefiles (I use them extensively in other non-java projects) but it would be helpful to have more feedback on why adding this would be useful to this project specifically.

  • The command to build (without testing) is slightly shorter (make build vs mvn install -DskipTests), but the command to test is slightly longer (make test vs mvn test). Generally I'd expect someone who is building locally to be doing so in order to test that their changes work, and mvn install is shorter than make build && make test.
  • This would make it marginally easier for people who are unfamiliar with maven (and who are familiar with make) to understand the build process, but it doesn't change the requirement to have maven installed, maven isn't very complicated, and the entire project and its ci settings are built around maven
  • This would require future contributors to learn makefiles if they ever wanted to make changes to the build process, as they'd have to edit the maven files as well as the makefile (I've worked with projects that have had multiple parallel methods for building, and from my experience it leads to inconsistency).
  • This project in general doesn't need encouragement to build from source, as:
    • Someone that is working a pull request undoubtedly is using an IDE that supports java, and I'm fairly certain all popular java IDEs support maven
    • Someone looking for a build to run should use the existing releases, as master is not guaranteed to work, and it's more difficult to debug builds from unknown sources.

I'm open to discussion about the merits of adding this, if you or anyone else would like to weigh in.

@Chopper1337
Copy link
Author

My thinking was mostly this: If build instruction aren't going to be provided, it's a good enough way to indicate how to build it.

Personally, I had forgotten how to build Java applications and had added this such that I wouldn't need to remember how to :-)

If not a Makefile, maybe a small section on building and testing could be added to README.md instead?

@jagrosh
Copy link
Owner

jagrosh commented Apr 10, 2024

maybe a small section on building and testing could be added to README.md instead?

I'd rather do this than add a Makefile, although I'm still hesitant to encourage people to build the project themselves, as someone who is capable of making a pull request should already be able to open and build this project in their IDE (without any additional changes/instructions; most Java IDEs support maven), and I don't want to nor have the time to help people who run into issues trying to build it on their own.

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.

2 participants