search

repaint-io / maven-tiles

Injecting maven configurations by composition rather than inheritance
154 stars 32 forks source link

Less strict restriction for reactor projects using tiles #32

Closed Gawen-pjr closed 9 years ago

Gawen-pjr commented 9 years ago

Fail build only if the reactor is used as parent POM by one of its module. Also successfully tested partial builds with -pl, -am and -amd flags.

talios commented 9 years ago

I've merged and re-pushed for gerrithub reviews:

https://review.gerrithub.io/249196 Less strict restriction for reactor projects using tiles

I've left the commit author as @Gawen- for attribution purposes.

talios commented 9 years ago

Included in the newly release 2.3.

Gawen-pjr commented 9 years ago

Thanks for the tips. I'll use gerrithub next time.

Gawen-pjr commented 9 years ago

Hello again,

You forgot to remove the else section arround orchestrateMerge(currentProject). The method call must simply be the last statement of the if (containsTiles) block. With the else in place, a reactor using tile but not used as parent will simply silently do nothing without throwing an exception, which is not the desired behavior.

PS : I failed to sign in Gerrithub with my Github account. Don't know why... So I cannot push the patch the way you indicated for the moment

Edit : The problem was the trailing dash in my login... Changed it and now it works. I will repush the changes I was taking about soon.

Gawen-pjr commented 9 years ago

Changes made here : https://review.gerrithub.io/#/c/249602

Hope I did it the right way this time ;-)

talios commented 9 years ago

Yep! Saw it, added some comments. My bad for missing the else block. Rather than maybe creating a new ticket you could just refer back to this one.

Gawen-pjr commented 9 years ago

Updated : https://review.gerrithub.io/#/c/249602

Thanks for your advises. This is my first open-source effective contribution, and this experience will bring me a lot of good practices to apply internally in my company.