Skip to content

Conversation

@hboutemy
Copy link
Member

@hboutemy hboutemy commented Jan 9, 2023

@hboutemy
Copy link
Member Author

uh, I'm surprised that commit 38fc24b creates an IT failure...

next steps:

@asfgit asfgit force-pushed the MNG-7664 branch 2 times, most recently from d15cc3f to c8e8d79 Compare January 15, 2023 10:41
@hboutemy
Copy link
Member Author

hboutemy commented Jan 15, 2023

at this stage:

@hboutemy
Copy link
Member Author

here it is: everything is shared in root module
it requires a small update in Modello: codehaus-plexus/modello#292

the 2 specific generator Velocity templates are named model-v3-modified.vm and reader-modified.vm, waiting for work to reintegrate modification in original templates

@hboutemy hboutemy requested review from gnodet and michael-o January 15, 2023 18:23
Copy link
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't be shared resources module better than ${basedir}/.. magic?

@gnodet
Copy link
Contributor

gnodet commented Jan 16, 2023

Wouldn't be shared resources module better than ${basedir}/.. magic?

What would be the benefit of creating an additional module and publishing the artifact if it's purely internal ?
Also, I think the fact that velocity templates are used means that the templates need to be on the filesystem, which means extracting them before running modello. That sounds a bit complicated imho...

@michael-o
Copy link
Member

Wouldn't be shared resources module better than ${basedir}/.. magic?

What would be the benefit of creating an additional module and publishing the artifact if it's purely internal ? Also, I think the fact that velocity templates are used means that the templates need to be on the filesystem, which means extracting them before running modello. That sounds a bit complicated imho...

First of all, the deployment can be skipped. I remember that in this exact situation we recommend users to share a module in reactor instead of dealing with path traversal. Of course, it is a bit more work. The result is the same. I will leave the decision to @hboutemy .

@hboutemy
Copy link
Member Author

hboutemy commented Jan 17, 2023

thinking again at Modello Velocity Plugin, instead of magically reusing existing basedir parameter, probably creating a new dedicated velocityBasedir directory could make things more clear
and this could even differentiate basedir for .mdo from basedir for .vm, then have better sharing strategies for each (.vm in root, but .mdo in modules even if also loaded from another module)
I'll test that tonight

@hboutemy
Copy link
Member Author

done: I'm now happy with the result:

  • .mdo files stay at their original location, reuse from the 3 api-oriented ones is done without attach but direct access
  • .vm files are used directly from root src/mdo directory

last review before I work on doing the Modello 2.1.1 release that includes necessary changes then I can merge this PR

@hboutemy hboutemy merged commit 74548dd into master Jan 19, 2023
@hboutemy hboutemy deleted the MNG-7664 branch January 19, 2023 23:38
@gnodet gnodet added this to the 4.0.0-alpha-4 milestone Jan 20, 2023
@jira-importer
Copy link

Resolve #8788

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.

4 participants