You are viewing a plain text version of this content. The canonical link for it is here.
Posted to easyant-dev@incubator.apache.org by Nicolas Lalevée <ni...@hibnet.org> on 2011/08/06 14:56:23 UTC
Clean up of some IP issues - Round 2
So we did a first clean up. And we did more review on the diff-to-device.txt
So here is the second round of the remaining stuff to clean up. Still 3 categories:
* to keep. Please mentors review the diff-to-keep-2.txt
* to remove. Please developers review diff-to-revert-2.txt and raise issues if it will dramatically break easyant. The plugin build-tool will also be removed.
* and the remaining IP to clean up : diff-to-decide-2.txt
About the remaining stuff (diff-to-decide-2.txt), here are the remaining questions:
* r239,207,205 : documentation and css : can we consider there not much IP there ?
* r487 : performance improvement of module resolution : if it's just performance improvement, it's not critical to remove it, right ? We could improve the performance later
* r485,484 : jar resolver : can we imagine we get rid of the jar resolver and implement a zip resolver ?
* r428 : package-test-jar : Maybe we can get rid of the existing package-test-jar and reimplement it ?
Nicolas
Re: Clean up of some IP issues - Round 2
Posted by Jean-Louis Boudart <je...@gmail.com>.
Sorry for the delay, i was on vacations.
Le 23 août 2011 14:29, Nicolas Lalevée <ni...@hibnet.org> a écrit
:
>
> > - 487 (this is not only performance, the change to RepositoryReport
> > adds support for a selectable resolver, or so it seems).
>
> It seems the key point in the performance improvement is about not
> searching in every resolver, so the selectable resolver. Am I right, do you
> confirm Jean-Louis ?
> And since I suggest later to get rid of the RepositoryReport, we would be
> OK to just revert it.
>
You're right 487 is just a performance improvement, so we can revert it. We
could ask ourself the question : what is the purpose of RepositoryReport?
We were using it in easyant-core build itself to collect all metadata
(available targets, phases, properties etc...) from all "published" plugins.
It was also in charge to generate the plugin documentation through an inner
class (XSLReport) based on those metadata.
I think it's time to change how we generate plugin documentation as plugins
can have a different lifecycle from easyant-core.
>
> > - 484 and 485 - I'm not exactly sure what the JarResolver does, but it
> > applies to the jar: protocol only. It shouldn't be too hard to
> > rewrite that without looking at the current code if you can give a
> > proper description of its interface and expected behavior.
>
> Now I have looked closer to the current implementation, something seems
> wrong to me. The jar resolver seems to generically handle any jar url. So i
> can handle files in any jar. the pattern used could potentially make the jar
> we are looking into change. Seems to much and I think we can do simpler.
> Probably the jar resolver could handle only one jar, and the ivy pattern
> would be only about paths within the specified jar. So instead of specifying
> this:
> <jar name="easyant-core-modules">
> <ivy
> pattern="jar:file:/tmp/myjar.jar!/myrepo/[organisation]/[module]/[revision]/[module].ivy"/>
> </jar>
> We would write:
> <jar name="easyant-core-modules" jar="file:/tmp/myjar.jar">
> <ivy
> pattern="/myrepo/[organisation]/[module]/[revision]/[module].ivy"/>
> </jar>
>
> WDYT ?
> Ok if you reimplement it that way ?
>
Looks good for me
> And by the way, no objection if we write this resolver in Ivy instead of
> Easyant ?
>
Originally we were supposed to push JarResolver in ivy, so yes if we write
this resolver again it's probably better to do it there.
>
> > I happen to know the ZIP and JAR specs pretty well, BTW ;-)
>
> noted. Now every question I'll have about theses spec will be for you :)
>
> > But those same revisions also change quite a few things in the reporting
> > part (XslReport), I haven't completely figured out how much of this is
> > just a change in indentation and how mach changes functionality.
> >
> > If it is only indentation then revert the change and run your Eclipse
> > (or whatever you use) formatter again.
>
> humm, another thing I missed.
> I would say that since it is not important for easyant to run, you should
> get rid of this ant task. Objections ?
>
+1 to remove this task
>
> > - 428 revert and re-implement it from a spec of what it tests.
>
> Any objection if I just remove it for now ?
>
I think this commit has already been reverted, original author has moved the
package-test-jar functionnality in a dedicated plugin.
--
Jean Louis Boudart
Independent consultant
Apache EasyAnt commiter http://incubator.apache.org/easyant/
Re: Clean up of some IP issues - Round 2
Posted by Stefan Bodewig <bo...@apache.org>.
On 2011-09-04, Nicolas Lalevée wrote:
>> Now I have looked closer to the current implementation, something
>> seems wrong to me. The jar resolver seems to generically handle any
>> jar url. So i can handle files in any jar. the pattern used could
>> potentially make the jar we are looking into change. Seems to much
>> and I think we can do simpler.
>> Probably the jar resolver could handle only one jar, and the ivy
>> pattern would be only about paths within the specified jar. So
>> instead of specifying this:
>> <jar name="easyant-core-modules">
>> <ivy pattern="jar:file:/tmp/myjar.jar!/myrepo/[organisation]/[module]/[revision]/[module].ivy"/>
>> </jar>
>> We would write:
>> <jar name="easyant-core-modules" jar="file:/tmp/myjar.jar">
>> <ivy pattern="/myrepo/[organisation]/[module]/[revision]/[module].ivy"/>
>> </jar>
>> WDYT ?
>> Ok if you reimplement it that way ?
>> And by the way, no objection if we write this resolver in Ivy instead of Easyant ?
> I have implemented it in Ivy:
> http://ant.apache.org/ivy/history/trunk/resolver/jar.html
> So I guess we should try to use the Ivy one.
Great!
Stefan
Re: Clean up of some IP issues - Round 2
Posted by Nicolas Lalevée <ni...@hibnet.org>.
Le 23 août 2011 à 14:29, Nicolas Lalevée a écrit :
>
> Le 8 août 2011 à 17:21, Stefan Bodewig a écrit :
>
>> On 2011-08-06, Nicolas Lalevée wrote:
>>
>>> So we did a first clean up. And we did more review on the diff-to-device.txt
>>
>> A big thank you and sorry for not helping more or being more responsive.
>
> I'm not that responsive either. And your help is as always greatly appreciated.
>
>>
>>> So here is the second round of the remaining stuff to clean up. Still 3 categories:
>>> * to keep. Please mentors review the diff-to-keep-2.txt
>>
>> OK with me.
>>
>>> * and the remaining IP to clean up : diff-to-decide-2.txt
>>
>> Revs that look as if they should be removed to me:
>>
>> - 487 (this is not only performance, the change to RepositoryReport
>> adds support for a selectable resolver, or so it seems).
>
> It seems the key point in the performance improvement is about not searching in every resolver, so the selectable resolver. Am I right, do you confirm Jean-Louis ?
> And since I suggest later to get rid of the RepositoryReport, we would be OK to just revert it.
>
>> - 484 and 485 - I'm not exactly sure what the JarResolver does, but it
>> applies to the jar: protocol only. It shouldn't be too hard to
>> rewrite that without looking at the current code if you can give a
>> proper description of its interface and expected behavior.
>
> Now I have looked closer to the current implementation, something seems wrong to me. The jar resolver seems to generically handle any jar url. So i can handle files in any jar. the pattern used could potentially make the jar we are looking into change. Seems to much and I think we can do simpler.
> Probably the jar resolver could handle only one jar, and the ivy pattern would be only about paths within the specified jar. So instead of specifying this:
> <jar name="easyant-core-modules">
> <ivy pattern="jar:file:/tmp/myjar.jar!/myrepo/[organisation]/[module]/[revision]/[module].ivy"/>
> </jar>
> We would write:
> <jar name="easyant-core-modules" jar="file:/tmp/myjar.jar">
> <ivy pattern="/myrepo/[organisation]/[module]/[revision]/[module].ivy"/>
> </jar>
>
> WDYT ?
> Ok if you reimplement it that way ?
> And by the way, no objection if we write this resolver in Ivy instead of Easyant ?
I have implemented it in Ivy:
http://ant.apache.org/ivy/history/trunk/resolver/jar.html
So I guess we should try to use the Ivy one.
Nicolas
>
>> I happen to know the ZIP and JAR specs pretty well, BTW ;-)
>
> noted. Now every question I'll have about theses spec will be for you :)
>
>> But those same revisions also change quite a few things in the reporting
>> part (XslReport), I haven't completely figured out how much of this is
>> just a change in indentation and how mach changes functionality.
>>
>> If it is only indentation then revert the change and run your Eclipse
>> (or whatever you use) formatter again.
>
> humm, another thing I missed.
> I would say that since it is not important for easyant to run, you should get rid of this ant task. Objections ?
>
>> - 428 revert and re-implement it from a spec of what it tests.
>
> Any objection if I just remove it for now ?
>
>> I'm totally willing to be more lenient with docs and CSS changes so
>> would be OK with keeping the rest.
>
> ok.
>
> Nicolas
>
Re: Clean up of some IP issues - Round 2
Posted by Stefan Bodewig <bo...@apache.org>.
On 2011-08-23, Nicolas Lalevée wrote:
> Le 8 août 2011 à 17:21, Stefan Bodewig a écrit :
>> Revs that look as if they should be removed to me:
>> - 487 (this is not only performance, the change to RepositoryReport
>> adds support for a selectable resolver, or so it seems).
> It seems the key point in the performance improvement is about not searching in every resolver, so the selectable resolver. Am I right, do you confirm Jean-Louis ?
> And since I suggest later to get rid of the RepositoryReport, we would be OK to just revert it.
Getting rid of RepositoryReport sounds good to me.
Stefan
Re: Clean up of some IP issues - Round 2
Posted by Nicolas Lalevée <ni...@hibnet.org>.
Le 8 août 2011 à 17:21, Stefan Bodewig a écrit :
> On 2011-08-06, Nicolas Lalevée wrote:
>
>> So we did a first clean up. And we did more review on the diff-to-device.txt
>
> A big thank you and sorry for not helping more or being more responsive.
I'm not that responsive either. And your help is as always greatly appreciated.
>
>> So here is the second round of the remaining stuff to clean up. Still 3 categories:
>> * to keep. Please mentors review the diff-to-keep-2.txt
>
> OK with me.
>
>> * and the remaining IP to clean up : diff-to-decide-2.txt
>
> Revs that look as if they should be removed to me:
>
> - 487 (this is not only performance, the change to RepositoryReport
> adds support for a selectable resolver, or so it seems).
It seems the key point in the performance improvement is about not searching in every resolver, so the selectable resolver. Am I right, do you confirm Jean-Louis ?
And since I suggest later to get rid of the RepositoryReport, we would be OK to just revert it.
> - 484 and 485 - I'm not exactly sure what the JarResolver does, but it
> applies to the jar: protocol only. It shouldn't be too hard to
> rewrite that without looking at the current code if you can give a
> proper description of its interface and expected behavior.
Now I have looked closer to the current implementation, something seems wrong to me. The jar resolver seems to generically handle any jar url. So i can handle files in any jar. the pattern used could potentially make the jar we are looking into change. Seems to much and I think we can do simpler.
Probably the jar resolver could handle only one jar, and the ivy pattern would be only about paths within the specified jar. So instead of specifying this:
<jar name="easyant-core-modules">
<ivy pattern="jar:file:/tmp/myjar.jar!/myrepo/[organisation]/[module]/[revision]/[module].ivy"/>
</jar>
We would write:
<jar name="easyant-core-modules" jar="file:/tmp/myjar.jar">
<ivy pattern="/myrepo/[organisation]/[module]/[revision]/[module].ivy"/>
</jar>
WDYT ?
Ok if you reimplement it that way ?
And by the way, no objection if we write this resolver in Ivy instead of Easyant ?
> I happen to know the ZIP and JAR specs pretty well, BTW ;-)
noted. Now every question I'll have about theses spec will be for you :)
> But those same revisions also change quite a few things in the reporting
> part (XslReport), I haven't completely figured out how much of this is
> just a change in indentation and how mach changes functionality.
>
> If it is only indentation then revert the change and run your Eclipse
> (or whatever you use) formatter again.
humm, another thing I missed.
I would say that since it is not important for easyant to run, you should get rid of this ant task. Objections ?
> - 428 revert and re-implement it from a spec of what it tests.
Any objection if I just remove it for now ?
> I'm totally willing to be more lenient with docs and CSS changes so
> would be OK with keeping the rest.
ok.
Nicolas
Re: Clean up of some IP issues - Round 2
Posted by Stefan Bodewig <bo...@apache.org>.
On 2011-08-06, Nicolas Lalevée wrote:
> So we did a first clean up. And we did more review on the diff-to-device.txt
A big thank you and sorry for not helping more or being more responsive.
> So here is the second round of the remaining stuff to clean up. Still 3 categories:
> * to keep. Please mentors review the diff-to-keep-2.txt
OK with me.
> * and the remaining IP to clean up : diff-to-decide-2.txt
Revs that look as if they should be removed to me:
- 487 (this is not only performance, the change to RepositoryReport
adds support for a selectable resolver, or so it seems).
- 484 and 485 - I'm not exactly sure what the JarResolver does, but it
applies to the jar: protocol only. It shouldn't be too hard to
rewrite that without looking at the current code if you can give a
proper description of its interface and expected behavior.
I happen to know the ZIP and JAR specs pretty well, BTW ;-)
But those same revisions also change quite a few things in the reporting
part (XslReport), I haven't completely figured out how much of this is
just a change in indentation and how mach changes functionality.
If it is only indentation then revert the change and run your Eclipse
(or whatever you use) formatter again.
- 428 revert and re-implement it from a spec of what it tests.
I'm totally willing to be more lenient with docs and CSS changes so
would be OK with keeping the rest.
Stefan