Hi Francisco, many thanks for your comment, I was going to write exactly the same things, moreover because some comparison made are extremely unfair and does not enter the real content, but only the external surface 😅 As I wrote, I do not like much the other solution proposed because IMO it is much more cumbersome and error-prone to maintain longish configurations that depends mostly on sort of "chance" - i.e. combination of flags and order of writing - than writing ad-hoc code that by-design provides the required behavior. The copy'n'paste issue could have simply been solved using programmatically the invoker plugin (a thing I already did in other plugins, sort of "standard" for such use cases). Anyway, there are multiple cases that would not be covered by my too-naive approach. It would require a longish refactoring to ensure that all of them would be correctly managed, so I preferred to close it, since we have an alternative, and keep going on with more urgent tasks. Reason I mention that is because I did not see any comment about what are the real problems behind my solution, why and in which cases it could fail, etc etc. And, IMO, this is a symptom of sort of "superficiality" in correct evaluation of pros/cons of different alternatives, that often are based only on abstract and generic concepts and do not really go inside the matter. I write this reply only as input/suggestion for the future, and the discussion for this specific PR is settled, for me.
M2C Cheers Il giorno mer 11 giu 2025 alle ore 11:49 Francisco Javier Tirado Sarti <[email protected]> ha scritto: > Hi Tibor, > I think both PRs have merit. > Yours is a kind of simpler workaround for that particular issue. So my > kudos ;) > Gabriele's is the almost optimal solution from my point of view (except > for the missing simplification to reuse the copy-pasted code, which would > have been my comment in that PR ;)) And it is potentially usable in > other scenarios we might have in the future, so my kudos too ;) > Now, being controversial. Taking into account (if I understood correctly > the time sequence) that Gabriele's PR was already there when you opened > yours, I agree with Gabriele that the second one was probably not needed. > Also, more globally and not strictly related to this issue, we should not > "fear" adding extra code. It is a recurrent comment that adding lines of > code is a negative thing, like if maintaining code by people used to > maintaining code was a problem. It should not be, as long as the code is > fine and has a purpose, which is therothetically guaranteed by code > reviews. > Which is a problem is to maintain cumbersome code that nobody is able to > fully understand or to keep dead code that is not used by anyone, > Gabriele's PR does not enter into any of these two categories, so I hardly > see how a supposed "maintenance problem" can be an argument against his PR. > Now, being practical. Since Gabriele's PR is already closed and yours is > opened and approved, let's move forward and merge it. If, in future, we > need multiple URLs again, then we should recover Gabriele`s PR and use his > approach. > > > On Wed, Jun 11, 2025 at 9:35 AM Tibor Zimányi <[email protected]> wrote: > > > Hi, > > > > I see Gabriele closed his PR and there is feedback from one person (thank > > you Alex for the feedback). Does anyone else have opinions about this, > > please? > > > > If it is fine to continue with my PR (as Gabriele decided to close his), > > could someone please review my PR, if it is fine? I see Yeser already > > approved, however based on the guidelines, there needs to be more > > reviewers. > > > > Best regards, > > Tibor > > > > On Tue, Jun 10, 2025 at 10:44 AM Alex Porcelli <[email protected]> > > wrote: > > > > > Hi all, > > > > > > First of all, thank you both, Tibor and Gabriele, for the work and > > > time you put into this. > > > > > > I understand that Gabriele's solution aims to simplify the > > > configuration, which is appreciated. However, it introduces around > > > ~1,000 lines of custom plugin code, which brings additional > > > maintenance overhead. Based on our experience with build-chain, > > > investing in code outside the core Apache KIE domain is something we > > > should approach with caution. > > > > > > Tibor's approach, although more verbose in configuration, utilizes > > > only standard tools and does not require additional code for > > > maintenance. The concern is not just about writing more code, but > > > about maintaining code that falls outside the core scope of the > > > project (such as Maven plugin logic for downloading resources). > > > > > > Given that, I advocate for and support Tibor's approach as the > > > immediate path forward. > > > > > > At the same time, I'd encourage Gabriele to explore contributing the > > > enhancement to the original plugin. If accepted, we could adopt it and > > > simplify the configuration. > > > > > > Thanks again to both of you for your efforts. > > > > > > - > > > Alex > > > > > > On Tue, Jun 10, 2025 at 2:59 AM Tibor Zimányi <[email protected]> > > wrote: > > > > > > > > Hi everyone, > > > > > > > > for external XSD downloads for the DMN and BPMN standard, we have > Maven > > > > modules that downloads those XSDs and packs them into jars. Recently > > > there > > > > was a problem with the Drools build, because the OMG organisation had > > > some > > > > outage and the XSDs were not available to download, so the DMN XSDs > jar > > > > build was failing. To fix the problem, there was an effort to have > > > fallback > > > > URLs for download. For DMN, there is GitHub organisation of the OMG > RTF > > > > taskforce, that contains these XSDs. > > > > > > > > We have two proposed solutions, how to have the fallback downloads > > > > implemented. Gabriele implemented a custom Maven plugin, extending > the > > > > download-maven-plugin that we already use to download the XSDs. You > can > > > see > > > > it in his PR here (1). I myself found out that such fallback > downloads > > > are > > > > able to be configured without any custom code just using the > > > > download-maven-plugin we aready use. You can see my solution here > (2). > > > > There is some discussion about the proposed solutions in my PR, where > > > > Gabriele thinks my solution is not ideal and where I think Gabriele's > > > > solution is not ideal. > > > > > > > > Could you please review both solutions and give us feedback, which is > > the > > > > preferred one? > > > > > > > > Best regards, > > > > Tibor > > > > > > > > (1) https://github.com/apache/incubator-kie-drools/pull/6370 > > > > (2) https://github.com/apache/incubator-kie-drools/pull/6371 > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: [email protected] > > > For additional commands, e-mail: [email protected] > > > > > > > > >
