-
Notifications
You must be signed in to change notification settings - Fork 3
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
More restrictive dependencies for maven #31
base: master
Are you sure you want to change the base?
Conversation
@maybeec thanks for the PR. Minimizing dependencies is always a good thing. I can still extend them later when needed. I am trying to setup CI to verify PRs ind this repo and just added the action to master. |
CI issue: #32 |
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.
So adding CI prior to merging was a good step as it reveals that you did not test this change and it breaks the build.
So either we can close this PR or you can rework it so the build is green.
should be fixed now |
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.
seems fine. CI hangs. Trying to restart...
@maybeec thanks again for your PR 👍 |
I see it's even not a branch on my fork but on this repository. |
Ah, now I get it:
This PR was created from a fork that does not exist anymore. In the early days of Github this broke the PR. Nowadays when a PR is created Github creates an internal branch to keep the diff persistent for the repo. |
Maybe there is a need for including all maven-core, but I couldn't find it in my use case utilizing mmm java maven.
Possibly it's worth thinking about replacing maven-core with maven-model dependency if the rest of maven-core is not needed.