-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
MNG-7400 - Allow more WorkspaceReader's to participate #667
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.
Thanks a lot for contributing.
I'm not done yet with the review (I just need a bigger screen which I don't have available at this moment); I'd like to take a more thorough look soon.
In the meantime, would you be so kind to revert the unrelated change in imports? It allows us to focus on the actual changes you're proposing. Thanks!
Thanks for looking into this, there is currently an issue that this also finds the I'll try to cleanup the unrelated parts, it just seems the Eclipse-Formater likes to format more than desired here... |
53244e4
to
edbc026
Compare
Done! |
I have uploaded my "HelloWorld" here: https://github.com/eclipse/tycho/pull/586/files |
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.
Thanks for your patience. I was able to have a better look now and I left a couple of questions.
87773fc
to
fd2337a
Compare
@laeubi Please address the open comments before I am merge this. |
We at Tycho have a high demand in integrating better with maven to reduce some workarounds that where used in the past to get custom dependency types into the maven dependency model (namely using system scoped absolute paths).
For that reason it would help if it where possible to register additional WorkspaceReader to participate.
I have created a prototype to show/test this new feature.
FYI @mickaelistria @akurtakov
This PR allows maven-extensions to supply
WorkspaceReader
as it is possible withAbstractMavenLifecycleParticipant
currently it is only possible with some workarounds:AbstractMavenLifecycleParticipant
in a maven-core-extensionafterSessionStart
cast theRepositorySystemSession
toDefaultRepositorySystemSession
setWorkspaceReader
and take care not to replace an already exiting oneWith this change it is possible to simply register WorkspaceReader component directly and Maven takes care of all the rest.
Following this checklist to help us incorporate your
contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[MNG-XXX] SUMMARY
, where you replaceMNG-XXX
and
SUMMARY
with the appropriate JIRA issue. Best practice is to use the JIRA issuetitle in the pull request title and in the first line of the commit message.
mvn clean verify
to make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.