Re: M2 : car-maven-plugin and geronimo-plugin.xml files

[anita kulshreshtha]

inline..

--- Jason Dillon [EMAIL PROTECTED] wrote:

 On Aug 7, 2006, at 10:33 PM, anita kulshreshtha wrote:
  This code is from servlets-examples-jetty config (rev 429124):
 resources
  resource
  directory${pom.basedir}/src/conf/directory
  targetPathMETA-INF/targetPath
  includes
  includegeronimo-plugin.xml/include
  /includes
  filteringtrue/filtering
  /resource
  /resources
 
 This code has been added to many applications config. Which
 means
  that you are trying to write it yourself and have no intention of  
  using
  the patch.
 
 I was simply reusing the existing Maven2 resources plugin to handle  
 filtering of resources.


This high quality code does not do what it is supposed to do, i.e. put
geronimo-plugin.xml in the generated car.


 
 I looked over your patch and could not apply it directly due to the  
 number of other changes made to the tree since the patch was  
 originally crafted.
 
 
  Why did you ask me to make the patch?
 
 I asked you to roll new patches against m2migration and not off of  
 trunk so that I could quickly verify and apply them.


The patch on July 27th was for m2migration and it is clearly written. 



 
 
  Wow.. I don't blame you for exercising the power of a committer.
 you
  get to commit code that does nothing and reject the code that
 works!
  You have the power to shut down other peoples work.
 
 I am starting to take offense to some of these comments you are  
 making.  I'm not sure if you are trying to goat me into a conflict or
  
 if you are trying to resolve the work you have done and move forward.
 
 :-(
 
 
  Jason, I was also aware of the issues with the code and had been
  wanting to fix them and add more functionality. You are constantly
  changing the code that I wrote without any communication. You have 
 
  made
  it _impossible_ for me to work on this code. I am not saying that
 you
  are doing it intentionally.
 
 Since these commits end up with my user id attached to them, I am not
  
 willing to commit something that does not meet my standards for  
 quality.  I am not trying to invalidate your work, I am trying to get
  
 our m2 build functional and at the same time ensure a high standard  
 of quality for the code that supports it.


FYI, the code you are talking about was already committed by David
Jencks! David helped me write the plugin by expaining how the configs
work. He patiently reviewed massive patches, tested them, committed
them and made sure that the first server could be started. 


 
 
  IMO, you should have accepted the code
  because it provided the required functionality and allowed me to
 make
  improvements.
 
 The code submitted in the patches that I reviewed (and some that I  
 committed and then changed) were not using the Mojo API appropriately
  
 or effectively.  Just because a chunk of code works does not mean  
 that it should be blindly applied to the tree.


Isn't it because you added Mojo for the code that is not even being
used?


 
 I accepted the bulk of the code and cleaned it up to meet my  
 standards before I committed it.  Though some of your code I have not
  
 even begun to review since it is scattered amongst several issues and
  
 then into several patches in those issues, which makes it much harder
  
 for me to quickly verify and commit.
 
 Last time I checked the new patches are still using velocity and  
 custom file deletion bits instead of using the existing Plexus  
 support tools that handle this for you


The file deletion code is straight out of geronimo! If it is being used
in Geronimo, it is good enough for me. Why would I use Plexus?

BTW, many months ago *when you were not involved in this effort*, David
suggested that we should generate classpath dynamically. I was waiting
for the first server to start before I attempted that. That is why the
classPath was used as a massive string. It was only temporary.


 and nothing is commented. 
  
 So it is much more difficult for me to simply commit this.
 
 
  I agree with Hiram Chirino on this subject. I am quoting
  from a conversation on the list :
 

http://www.nabble.com/Re%3A--RTC--ActiveMQ-GBean-modules-p4867711.html
 
  Perhaps I should start a new thread on this thought, but I just  
  wanted
  to comment that we need to be careful about how critical and the
 level
  of perfection that we expect from the contributed patches.  I would
  say that if a patch does not regress the project and it moves it
  forward in the right direction, the patch should be accepted even
 if
  it's not perfect.
 
  It kind of reminds me of something David B told me once, if the
 code
  is perfect and stable, you won't be able to build a community
 around
  the project it since it just works.  This makes sense to me.  If
 the
  code is 80% of the way there, then you give an opportunity for
 folks
  to join 

Re: M2 : car-maven-plugin and geronimo-plugin.xml files

[Matt Hogstrom]

I get the sense that both of you are a bit frustrated.  The transition to the new RTC development 
model has been challenging for all.  The PMC has not kept up with the number of reviews and that has 
allowed the codebase to drift while patches then get stale.


Recently we've made several steps forward to improve the process.  Several new PMC members have been 
added in the last few weeks that are active committers to the project so the ability to provide 
timely feedback has been improved.  Along with some better mechanisms to track what needs to be 
reviewed so we can quickly address them.  (I think we got the pluggable JAAC completed last night).


All this to say it has made us stumble a bit in the process, we're not perfect yet, but we're making 
some important headway.


Everyone who has worked on the Maven 2 conversion needs to get some kudos as it is slightly larger 
than a bread box :) and given the somewhat binary nature of the change does not lend itself well to 
using patches as the vehicle to get the job done.


All that said, Jason, as your going through the patches and making changes what is the primary way 
to get feedback to the person who contributed the patch?  It sounds like you have some good feedback 
that would help Anita produce patches that you are both in more agreement on.  Also, it sounds like 
some of the changes are preferences based on style.  It would be a fair debate as to one should use 
Plexus or Geronimo infrastructure for the file delete activity.


All this said, you guys are to be commended for the progress you've made.  For the time being the 
review and collaboration feels like we've gone from a sprint to a jog but as we hit our stride I 
hope the pace will pick up as well.


anita kulshreshtha wrote:

inline..

--- Jason Dillon [EMAIL PROTECTED] wrote:


On Aug 7, 2006, at 10:33 PM, anita kulshreshtha wrote:

This code is from servlets-examples-jetty config (rev 429124):
   resources
resource
directory${pom.basedir}/src/conf/directory
targetPathMETA-INF/targetPath
includes
includegeronimo-plugin.xml/include
/includes
filteringtrue/filtering
/resource
/resources

   This code has been added to many applications config. Which

means
that you are trying to write it yourself and have no intention of  
using

the patch.
I was simply reusing the existing Maven2 resources plugin to handle  
filtering of resources.



This high quality code does not do what it is supposed to do, i.e. put
geronimo-plugin.xml in the generated car.


I looked over your patch and could not apply it directly due to the  
number of other changes made to the tree since the patch was  
originally crafted.




Why did you ask me to make the patch?
I asked you to roll new patches against m2migration and not off of  
trunk so that I could quickly verify and apply them.



The patch on July 27th was for m2migration and it is clearly written. 







Wow.. I don't blame you for exercising the power of a committer.

you

get to commit code that does nothing and reject the code that

works!

You have the power to shut down other peoples work.
I am starting to take offense to some of these comments you are  
making.  I'm not sure if you are trying to goat me into a conflict or
 
if you are trying to resolve the work you have done and move forward.


:-(



Jason, I was also aware of the issues with the code and had been
wanting to fix them and add more functionality. You are constantly
changing the code that I wrote without any communication. You have 
made

it _impossible_ for me to work on this code. I am not saying that

you

are doing it intentionally.

Since these commits end up with my user id attached to them, I am not
 
willing to commit something that does not meet my standards for  
quality.  I am not trying to invalidate your work, I am trying to get
 
our m2 build functional and at the same time ensure a high standard  
of quality for the code that supports it.



FYI, the code you are talking about was already committed by David
Jencks! David helped me write the plugin by expaining how the configs
work. He patiently reviewed massive patches, tested them, committed
them and made sure that the first server could be started. 






IMO, you should have accepted the code
because it provided the required functionality and allowed me to

make

improvements.
The code submitted in the patches that I reviewed (and some that I  
committed and then changed) were not using the Mojo API appropriately
 
or effectively.  Just because a chunk of code works does not mean  
that it should be blindly applied to the tree.



Isn't it because you added Mojo for the code that is not even being
used?


I accepted the bulk of the code and cleaned it up to meet my  
standards before I committed it.  Though some of your code I have not
 
even begun to review since it is scattered 

Re: M2 : car-maven-plugin and geronimo-plugin.xml files

[Jason Dillon]

On Aug 9, 2006, at 8:49 AM, Matt Hogstrom wrote:
Also, it sounds like some of the changes are preferences based on  
style.  It would be a fair debate as to one should use Plexus or  
Geronimo infrastructure for the file delete activity.


Are you kidding me?  Maven2 is built upon Plexus... Maven2 is a  
Plexus application.  Mojo's the Maven2 plugin system are also Plexus  
components.  All of the major plugins are using the support framework  
that Plaxus provides to handle these types of tasks.


There is absolutely no reason why we need to have duplicate code to  
delete files in our mojos.


This is not style, this is common sense and appropriate reuse of the  
Maven2 platform for our builds.


--jason

Re: M2 : car-maven-plugin and geronimo-plugin.xml files

[Jason Dillon]

On Aug 9, 2006, at 8:04 AM, anita kulshreshtha wrote:

The patch on July 27th was for m2migration and it is clearly written.


Where is it?  I keep getting lost in all of the patches.

--jason

Re: M2 : car-maven-plugin and geronimo-plugin.xml files

[Matt Hogstrom]

Well, you got me.  Given that so many people are heavily into Maven I have to admit ignorance in 
there.  I think Anita had made a comment that she was reusing existing Geronimo code and that you 
were using something from Plexus.


Not everyone has that same mighty mass of knowledge you take for granted :)  Thanks for the 
clarification.


Jason Dillon wrote:

On Aug 9, 2006, at 8:49 AM, Matt Hogstrom wrote:
Also, it sounds like some of the changes are preferences based on 
style.  It would be a fair debate as to one should use Plexus or 
Geronimo infrastructure for the file delete activity.


Are you kidding me?  Maven2 is built upon Plexus... Maven2 is a Plexus 
application.  Mojo's the Maven2 plugin system are also Plexus 
components.  All of the major plugins are using the support framework 
that Plaxus provides to handle these types of tasks.


There is absolutely no reason why we need to have duplicate code to 
delete files in our mojos.


This is not style, this is common sense and appropriate reuse of the 
Maven2 platform for our builds.


--jason

Re: M2 : car-maven-plugin and geronimo-plugin.xml files

[Jason Dillon]

On Aug 7, 2006, at 10:33 PM, anita kulshreshtha wrote:

This code is from servlets-examples-jetty config (rev 429124):
   resources
resource
directory${pom.basedir}/src/conf/directory
targetPathMETA-INF/targetPath
includes
includegeronimo-plugin.xml/include
/includes
filteringtrue/filtering
/resource
/resources

   This code has been added to many applications config. Which means
that you are trying to write it yourself and have no intention of  
using

the patch.


I was simply reusing the existing Maven2 resources plugin to handle  
filtering of resources.


I looked over your patch and could not apply it directly due to the  
number of other changes made to the tree since the patch was  
originally crafted.




Why did you ask me to make the patch?


I asked you to roll new patches against m2migration and not off of  
trunk so that I could quickly verify and apply them.




Vow.. I don't blame you for exercising the power of a committer. you
get to commit code that does nothing and reject the code that works!
You have the power to shut down other peoples work.


I am starting to take offense to some of these comments you are  
making.  I'm not sure if you are trying to goat me into a conflict or  
if you are trying to resolve the work you have done and move forward.


:-(



Jason, I was also aware of the issues with the code and had been
wanting to fix them and add more functionality. You are constantly
changing the code that I wrote without any communication. You have  
made

it _impossible_ for me to work on this code. I am not saying that you
are doing it intentionally.


Since these commits end up with my user id attached to them, I am not  
willing to commit something that does not meet my standards for  
quality.  I am not trying to invalidate your work, I am trying to get  
our m2 build functional and at the same time ensure a high standard  
of quality for the code that supports it.




IMO, you should have accepted the code
because it provided the required functionality and allowed me to make
improvements.


The code submitted in the patches that I reviewed (and some that I  
committed and then changed) were not using the Mojo API appropriately  
or effectively.  Just because a chunk of code works does not mean  
that it should be blindly applied to the tree.


I accepted the bulk of the code and cleaned it up to meet my  
standards before I committed it.  Though some of your code I have not  
even begun to review since it is scattered amongst several issues and  
then into several patches in those issues, which makes it much harder  
for me to quickly verify and commit.


Last time I checked the new patches are still using velocity and  
custom file deletion bits instead of using the existing Plexus  
support tools that handle this for you and nothing is commented.   
So it is much more difficult for me to simply commit this.




I agree with Hiram Chirino on this subject. I am quoting
from a conversation on the list :
http://www.nabble.com/Re%3A--RTC--ActiveMQ-GBean-modules-p4867711.html

Perhaps I should start a new thread on this thought, but I just  
wanted

to comment that we need to be careful about how critical and the level
of perfection that we expect from the contributed patches.  I would
say that if a patch does not regress the project and it moves it
forward in the right direction, the patch should be accepted even if
it's not perfect.

It kind of reminds me of something David B told me once, if the code
is perfect and stable, you won't be able to build a community around
the project it since it just works.  This makes sense to me.  If the
code is 80% of the way there, then you give an opportunity for folks
to join your community by submitting additional patches that help it
get to the 100% mark.


I generally agree with Hiram, though I don't think that we can allow  
build infrastructure related patches of diminished quality to be  
applied with out retrofitting them... or we will just make a larger  
mess for everyone to deal with.


 * * *

I am sorry that you are upset about the situation related to your  
patches.  I would really like for us to get past this and get back to  
being productive.


But, to be honest with you... the more defensive emails like this  
that you post, the less I want to continue working on the related  
issues.  I want to get the m2 work behind us and not get bogged down  
with conflict with those who are helping that work.


Again, I am sorry you are upset... but can we please try to move  
forward?


--jason

Re: M2 : car-maven-plugin and geronimo-plugin.xml files

[anita kulshreshtha]

--- Jason Dillon [EMAIL PROTECTED] wrote:

 The patch was
 not
  used, instead the plugin code was modified to add this
  functionality.
 
 I only added a few changes you had in the original patch since they  
 were the only ones that could be added cleanly.  
  ?
As you mentioned,  
 then functionality is still not there, so I really have issues with  
 the last sentence above.


This code is from servlets-examples-jetty config (rev 429124):
   resources
resource
directory${pom.basedir}/src/conf/directory
targetPathMETA-INF/targetPath
includes
includegeronimo-plugin.xml/include
/includes
filteringtrue/filtering
/resource
/resources
  
   This code has been added to many applications config. Which means
that you are trying to write it yourself and have no intention of using
the patch. Why did you ask me to make the patch? 

 But I can tell you right now that I will most lilly not take the  
 patch asis for the very same reasons why I had changed the plugin  
 before.

Vow.. I don't blame you for exercising the power of a committer. you
get to commit code that does nothing and reject the code that works!
You have the power to shut down other peoples work.

Jason, I was also aware of the issues with the code and had been
wanting to fix them and add more functionality. You are constantly
changing the code that I wrote without any communication. You have made
it _impossible_ for me to work on this code. I am not saying that you
are doing it intentionally. IMO, you should have accepted the code
because it provided the required functionality and allowed me to make
improvements. I agree with Hiram Chirino on this subject. I am quoting
from a conversation on the list :
http://www.nabble.com/Re%3A--RTC--ActiveMQ-GBean-modules-p4867711.html

Perhaps I should start a new thread on this thought, but I just wanted
to comment that we need to be careful about how critical and the level
of perfection that we expect from the contributed patches.  I would
say that if a patch does not regress the project and it moves it
forward in the right direction, the patch should be accepted even if
it's not perfect.

It kind of reminds me of something David B told me once, if the code
is perfect and stable, you won't be able to build a community around
the project it since it just works.  This makes sense to me.  If the
code is 80% of the way there, then you give an opportunity for folks
to join your community by submitting additional patches that help it
get to the 100% mark.
   
Thanks
Anita

__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

Re: M2 : car-maven-plugin and geronimo-plugin.xml files

[Jason Dillon]

Anita, I am aware of this issue.  I have not had time to resolve it  
completely.


More comments below...

On Aug 6, 2006, at 9:31 AM, anita kulshreshtha wrote:

The car-maven-plugin (trunk rev 429115) still does not add the
geronimo-plugin.xml file to the generated car. I had submitted a
working patch for the m2migration branch rev 425727 to include
geronimo-plugin.xml file to the zipped archive car. The patch was not
used, instead the plugin code was modified to add this
functionality.


I only added a few changes you had in the original patch since they  
were the only ones that could be added cleanly.  As you mentioned,  
then functionality is still not there, so I really have issues with  
the last sentence above.




I do believe in experiments when they work.


Which means?



I will be happy to elaborate on this, if anyone is interested.  Could
someone else (PMC member or committer) please explain to me why the
patch can not be used as is.


Do you really want me to critique the original sources?  I certainly  
can do that, but my feeling is that it will only add more fuel to  
this fire and leave both of us even more frustrated.  I have already  
commented on several major issues with the code in previous emails.




It is a few lines of code that uses
geronimo code to do its work, hence AFAICT this is not about not
trusting the code written by a non committer/non IBMer.


Hrm... what?  How does this have anything to do with IBM?


I am amazed at the amount of effort being spent to rewrite the  
plugin without giving

any technical reason as to why it needs to be modified in the first
place !


I have posted several technical reasons why I did not take your work  
asis in my response to the last inflammatory email you posted on this  
subject.  But, as I mentioned above if you would like a more  
comprehensive critique...




The original patch that added this functionality was submitted
on 19th June 06:
http://issues.apache.org/jira/browse/GERONIMO-2067#action_12416768

When GERONIMO-2225 was filed, it became clear that there was interest
in adding this functionality to the plugin. I resubmitted the patch
for the m2migration branch on 27th July, 06:
http://issues.apache.org/jira/browse/GERONIMO-2067#action_12416768


Give me a break... I was in the east coast for a week and then was  
camping for this weekend... and when I was working on m2migration I  
had several other issues to resolve on top of the issue which you  
supplied a patch for.




As expected the patch was not used, and a futile effort to rewrite the
code was made. I would really appreciate if some other committer/PMC
member could take a look at this code, and provide some technical
feedback about its shortcomings.


Yes you did submit a patch on July 27th... thank you.  I fully intend  
to dig into that issue more, I just have not had time to do so yet.   
But I can tell you right now that I will most lilly not take the  
patch asis for the very same reasons why I had changed the plugin  
before.


--jason

Re: M2 : car-maven-plugin and geronimo-plugin.xml files

[David Blevins]

Huh, I see you and Jason working *together* on the code.  I know  
you're not saying you don't want to work collaboratively with Jason  
and co, so maybe you can give some suggestions on how you both might  
collaborate better?


Just a comment about the whole no justification concept, that's  
kind of a tricky position to take.  On one hand discussion is good,  
on the other hand if you demand a justification for others code, it's  
an invite for others to start demanding the same of you.  That's  
pretty obvious and I'm probably not helping very much :)


I haven't said anything real useful, so why don't i just shut up and  
let you talk.  Taking this in a decidedly positive direction, what  
would you like to see more of out of you both?


-David


On Aug 6, 2006, at 9:31 AM, anita kulshreshtha wrote:


Jason and others,

The car-maven-plugin (trunk rev 429115) still does not add the
geronimo-plugin.xml file to the generated car. I had submitted a
working patch for the m2migration branch rev 425727 to include
geronimo-plugin.xml file to the zipped archive car. The patch was not
used, instead the plugin code was modified to add this
functionality. I do believe in experiments when they work. The new
code does not add the geronimo-plugin.xml file to the car file. Please
see why the code added by Jason does not work :
http://issues.apache.org/jira/browse/GERONIMO-2225#action_12423320

I will be happy to elaborate on this, if anyone is interested.  Could
someone else (PMC member or committer) please explain to me why the
patch can not be used as is. Did it not put the geronimo-plugin.xml
file in the car correctly?  It is a few lines of code that uses
geronimo code to do its work, hence AFAICT this is not about not
trusting the code written by a non committer/non IBMer. I am amazed at
the amount of effort being spent to rewrite the plugin without giving
any technical reason as to why it needs to be modified in the first
place ! The original patch that added this functionality was submitted
on 19th June 06:
http://issues.apache.org/jira/browse/GERONIMO-2067#action_12416768

When GERONIMO-2225 was filed, it became clear that there was interest
in adding this functionality to the plugin. I resubmitted the patch
for the m2migration branch on 27th July, 06:
http://issues.apache.org/jira/browse/GERONIMO-2067#action_12416768

As expected the patch was not used, and a futile effort to rewrite the
code was made. I would really appreciate if some other committer/PMC
member could take a look at this code, and provide some technical
feedback about its shortcomings.

Thanks

Anita

__
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around
http://mail.yahoo.com