-
Notifications
You must be signed in to change notification settings - Fork 241
chore: Simplify project by removing the Ant-based build; Maven only is fine #254
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR simplifies the project build system by removing the Ant-based build configuration and retaining only Maven for building and testing. The change removes legacy build infrastructure that was causing confusion, as mentioned in issue #253.
Changes:
- Removed Ant build configuration file (
build.xml) - Removed Ant build and test steps from the CI workflow
- Removed JAR dependencies from the
lib/directory that were used by the Ant build - Fixed trailing whitespace in the CI workflow file
Reviewed changes
Copilot reviewed 2 out of 8 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| build.xml | Complete removal of the Ant build configuration file |
| .github/workflows/ant.yml | Removed Ant build and test steps; retained Maven-based testing |
| lib/*.jar | Removed JAR dependencies (multibase, multiaddr, multihash, cid, junit, hamcrest) that were managed by Ant |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Yep totally fine. |
|
You should have also updated the readme file, because it still refers to Ant commands. Also, the CI workflow file should probably be renamed. |
| - name: Install and run ipfs | ||
| run: ./install-run-ipfs.sh | ||
| - name: Build & run tests with Maven | ||
| run: mvn test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it enable batch mode with -B and run verify instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM; see #262.
Triggered by noticing the confusion on #253.
@cstamas I'm sure you're in favour of this... 🤣
@ianopolous would this be cool with you?