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

Update Maven Model to 3.5.3 #12

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

Conversation

gastaldi
Copy link

@gastaldi gastaldi commented Apr 3, 2018

No description provided.

@gastaldi
Copy link
Author

gastaldi commented Apr 3, 2018

@jdcasey can you review it?

@jdcasey
Copy link
Member

jdcasey commented Apr 4, 2018

@rnc Are you okay with refactoring PME to use jdom2? That seems like it might be a significant change...

@gastaldi I think we need to do some looking into what this will affect. It's not the change in this project so much as the change to the jdom dep that will be complicated.

We might even need to branch the project to support both, then plan a migration path off of the old jdom.

@rnc
Copy link
Contributor

rnc commented Apr 4, 2018

@jdcasey Actually I think the README is out of date - the project already uses 2.0.6. Further PME has a direct dependency on jdom 2.0.6 so I think we should be fine :-)

@gastaldi
Copy link
Author

gastaldi commented Apr 4, 2018

@jdcasey as @rnc said, that is just the readme, JDom2 is already used in the pom.xml, I haven't changed that :)

@gastaldi
Copy link
Author

gastaldi commented Apr 4, 2018

Just realized that mvn clean install -Pgenerate is failing with

[ERROR] Failed to execute goal org.codehaus.modello:modello-maven-plugin:1.4.1:jdom-writer (toolchains) on project maven3-model-jdom-support: Execution toolchains of goal org.codehaus.modello:modello-maven-plugin:1.4.1:jdom-writer failed: There is no class 'TrackableBase' in the version range '1.0.0'. -> [Help 1]

This profile is still used, right?

@gastaldi
Copy link
Author

gastaldi commented Apr 4, 2018

Commit d0627ff fixes the error above, but fails to compile the classes since the generated code uses JDom 1.x

@gastaldi
Copy link
Author

gastaldi commented Apr 4, 2018

I removed the README changes and rebased the PR

@gastaldi
Copy link
Author

gastaldi commented Apr 4, 2018

@jdcasey btw, I just noticed that you were the one who introduced JDOM2: 1ad92b7 😄

@gastaldi
Copy link
Author

gastaldi commented Apr 4, 2018

Although it would be really nice to move back to using Jdom 1.x, since Maven still uses that

@jdcasey
Copy link
Member

jdcasey commented Apr 4, 2018

@gastaldi Ok, it's been awhile, I guess I lost track.

Looking at your comments, I think we might run into problems with code fixes that have been applied since the model was generated. We'll need to get the generate stuff running, then re-apply patches that were committed since the last time we range generate. This will probably be a matter of looking back through the commit log until we hit a point where there were huge changes to the model classes, and then analyzing changes since.

I have a feeling we haven't regenerated this project since initially committing the generated model to the Git repo.

@gastaldi
Copy link
Author

gastaldi commented Apr 4, 2018

That's fine, feel free to reject this PR if necessary.

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.

3 participants