search

eclipse-oomph / oomph

Eclipse Public License 2.0
6 stars 11 forks source link

Add DynamicMavenProjectFactory to deal with tycho-pomless #62

Closed merks closed 10 months ago

merks commented 10 months ago

Fixes https://github.com/eclipse-oomph/oomph/issues/55

merks commented 10 months ago

@kysmith-csg

I'm not sure if you want to retest, but I think these changes are good to go. It was great that you provided a sample repository because that really helped me understand they implementation details!!

kysmith-csg commented 10 months ago

@merks You should've just been able to push to my fork, I'm pretty sure I checked the box that says "allow maintainers to contribute". But anyways, I'm not sure what all your changes do exactly, but it still seems to import the projects correctly so I think it's ok!

I was worried at first when I saw an icon/ui things because users shouldn't be able to actually add this as a task in their setup models, but I see that is still not the case so all is good. During the setup process I never saw the icon so I'm not entirely sure where it should appear.

merks commented 10 months ago

I'm not sure if we display these things in any views, but we could and there is a .edit project so it should be kept up-to-date with the model which happens when one does generate all for the *.genmodel. I also changed the model so that the XMLFileName is an actual attribute that could be serialized and would be shown in the properties view. It's just more consistent but not substantially different from what you did in terms of visible API. Then finally the version tool wanted version increments, so lots of noise from that too. I did read https://gist.github.com/wtbarnes/56b942641d314522094d312bbaf33a81 to figure how I might update your fork, but then I got lazy. Sorry about that. 😦

In any case, I am impressed that you got this all done without asking lots of question. Very impressed! 🏆 I'm happy with the result which only required a little bit of tweaking.

kysmith-csg commented 10 months ago

Having extensive experience with m2e/emf in our own product helps a lot ;)

Thank you for merging this so quick!

merks commented 10 months ago

I see. That explains a lot!! FYI, there will be a milestone build tomorrow so that changes will be generally available at that point.