geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Hogstrom <>
Subject Re: M2 : car-maven-plugin and geronimo-plugin.xml files
Date Wed, 09 Aug 2006 15:49:07 GMT
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

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

Everyone who has worked on the Maven 2 conversion needs to get some kudos as it is slightly
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
to get feedback to the person who contributed the patch?  It sounds like you have some good
that would help Anita produce patches that you are both in more agreement on.  Also, it sounds
some of the changes are preferences based on style.  It would be a fair debate as to one should
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
review and collaboration feels like we've gone from a sprint to a jog but as we hit our stride
hope the pace will pick up as well.

anita kulshreshtha wrote:
> inline..
> --- Jason Dillon <> 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>
>>>                 <targetPath>META-INF</targetPath>
>>>                 <includes>
>>>                     <include>geronimo-plugin.xml</include>
>>>                 </includes>
>>>                 <filtering>true</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 :
>>> "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?
> Unfortunately we will have to revisit this until all the configs have
> the required functionality! You see, When they don't work people file
> issues. I am waiting patiently... 
> Thanks
> Anita
>> --jason
> __________________________________________________
> Do You Yahoo!?
> Tired of spam?  Yahoo! Mail has the best spam protection around 

View raw message