-
Notifications
You must be signed in to change notification settings - Fork 27
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
[MSHARED-1080] Parent POM 36, Java8, drop legacy. #38
Conversation
@@ -105,8 +110,6 @@ public void copyFile( File from, File to, boolean filtering, List<FileUtils.Filt | |||
} | |||
FileUtils.copyFile( from, to, encoding, new FileUtils.FilterWrapper[0], overwrite ); | |||
} | |||
|
|||
buildContext.refresh( to ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't something used by m2e? or not anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comes from a repo that is last touched 12 years ago, and is archived. IMHO is dead, parrot dead, but we can tinker about it if you want 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a quick search on github shows this https://github.com/search?q=org%3Aeclipse-m2e+BuildContext&type=code
for sure this very old repo is just interface which doesn't need much changes so that's probably the reason.
disclaimer i'm not using m2e so... 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are missing the point: this parrot is dead. Very. Also, whatever it is, this was first attempt to implement incremental build, followed by two other attempts (by same person), that finally materialized in something completely different. So, yes, this may look "stable", the very same way dead parrot looks "stable" (well, not moving, is it?), but is superseded by several other attempts...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe but I can see references of it in the m2e code.
Is it really used or not? I have no idea as I'm not a contributor of m2e.
My opinion is just it worth to ask those folks as we may or not break something.
if not perfect but it worth the courtesy of simply asking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not a rant. you point to a repo mentioning it's the logical successor.
whereas my point is by removing this you will affect all m2e users which is probably hundreds thousands even million(s) of users.
By pointing the repo I'm just saying m2e has a lot of contribution/maintenance and users compared to the repo you mention.
Even if I agree this was a bad idea/design (and as the guy who started the maven-filtering I didn't like it long time ago neither but it has been done...) but now it's here and use by a lot of people.
so we live with that or we discuss with m2e folks a replacement solution or at least ask them what are the impacts removing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I get it correctly, what you say is "buildcontext prevails over takari/incrementalbuild as m2e has more commits"? 🦘 LOL, well, I am about to stop here.... 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I get it correctly, what you say is "buildcontext prevails over takari/incrementalbuild as m2e has more commits"? 🦘 LOL, well, I am about to stop here.... 😆
no sorry but you do not read correctly, the most important is the number of users impacted.
and here in m2e is a lot!
what is better it's not the question here.
again I'm just saying the impacted users number is really huge!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
frankly looking at commit history m2e has more maintenance than https://github.com/takari/io.takari.incrementalbuild (last commit almost 2 yo ago).
And who said this then?
Anyway, lets bury the boomerang, sisu build api is raised from the dead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and my 5 cents to whole this topic: I never saw a "plain" (explained later) Maven project that is fairly more complex than "hello world" and works OOTB in m2e. Work, as in no "tweaks", "m2e profile in POM" needed etc. OTOH, I did saw Maven project using takari lifecycle that WORK perfectly and OOTB in m2e.
But alas, takari lifecycle use the "inferior" incrementalbuild and not buildcontext API. Oh, and it also replaces many things from Maven itself, and even ditches this maven-filtering and replaces it's with it's own to be functional 😄
("plain" as Maven build w/o any extension, while takari-lifecycle is an extension and replaces a LOT from maven, like core copmponets but also shared stuff like this maven-filtering and many plugins as well)
@fbricon still active m2e? any idea if this BuildContext is still in use? or maybe you can ping someone else? thanks |
@rakus ping |
src/main/java/org/apache/maven/shared/filtering/DefaultMavenFileFilter.java
Show resolved
Hide resolved
src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java
Show resolved
Hide resolved
So, given the m2e team response (or better: lack of it), I think that maven-filtering is actually not in use in m2e (is being completely replaced), or, that buildcontext seems totally irrelevant @olamy |
Sorry for the late response (I'm not super active on the m2e front), but yeah the buildcontext API is very much used by m2e: https://github.com/eclipse-m2e/m2e-core/search?q=BuildContext |
@fbricon but m2e uses this, very this implementation? Or it depends, as AFAIK when takari-lifecycle used, then this implementation is totally swapped out and unused by m2e. |
m2e only uses org.sonatype.plexus.build.incremental.BuildContext. Looks like the eclipse-takari integration uses its own thing (io.takari.incrementalbuild.BuildContext). @ifedorenko or @jvanzyl would know better. |
Changes:
https://issues.apache.org/jira/browse/MSHARED-1080