openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Craig L Russell <Craig.Russ...@Sun.COM>
Subject Re: svn commit: r569253 - in /openjpa/branches/1.0.0/openjpa-kernel/src/main: java/org/apache/openjpa/enhance/PCEnhancer.java resources/org/apache/openjpa/enhance/localizer.properties
Date Tue, 28 Aug 2007 01:43:34 GMT

On Aug 27, 2007, at 4:09 PM, Patrick Linskey wrote:

>> Granted, some of this discussion is with 20/20 hindsight.  But, we  
>> need to
>> figure out our approach and be consistent from the start.  All of  
>> us have
>> had various experiences with creating and maintaining branches and
>> releases.  We just need to find a common ground for us to live with.
>
> It sounds a whole lot like Kevin and Craig feel that I somehow made
> the process not work properly.

No, what I assume is that you had permission from Marc to check in  
the changes you made during the release cycle.

> Can we cite a problem that occurred
> based on what I did? If not, then can we alternately cite any concrete
> reason why what I did was undesirable? If not, then why are we
> spending all this time going back and forth about this topic?

Because I'm of the opinion that the RM should be able to say "I'm  
working on the release. Please no checkins until after the release is  
built."
>
>> I must really be missing something here, but why would we merge  
>> the contents
>> of the 1.0.0 branch back into trunk after the release has been  
>> approved?
>
> Typically, in the process of assembling a release, there will be
> changes that have to be made.

Of course.

> For example, I noticed that one of the
> first RCs was still shipping with the now-unnecessary JCA jars. In my
> experience, there are often quite a number of small tweaks that have
> to be made, and it is typically far easier to make all of these in one
> place and then copy them all back to trunk at one time, rather than
> maintaining two branches all along throughout the process.

That's fine, as long as the RM is willing to then merge the changes  
back to the trunk. But I don't believe that the RM should be required  
to do this on behalf of everyone who feels that they want to work on  
the release branch while the release is being stabilized.
>
> I think that maybe the big disconnect here is that Craig and Kevin
> seem to be trying to design something that prevents something from
> going wrong during the release process,

Yes.

> whereas I feel it's more
> appropriate to design a system that is easy to handle in the happy
> case.

In my own experience, a project as big as this one can use some help  
during release to avoid unnecessary respins simply because a late- 
breaking change was not perfect.

In this case, the only anomaly was Kevin's checkin that missed a  
respin. If there had been better communication between Kevin and  
Marc, this could have been avoided. Obviously this isn't a big deal  
because the release seems to be ok. And I trust that someone will  
take responsibility for merging all the 1.0.0 changes into the trunk.

> In this situation, the happy case is where people do not commit
> things to the release branch that shouldn't be in there. Assuming that
> that happens, then letting multiple people check into the release
> branch helps the release manager by decreasing the patch-moving
> workload, and does not incur any additional cost that I am aware of.

The critical concept here is "help the release manager". Assuming  
they want help. I'm ok with delegating this decision to the RM, but  
not mandating it.
>
>> should not have been put into the 1.0.0 branch.  At a minimum,  
>> Patrick's
>> changes should have been put into trunk and optionally into the  
>> 1.0 branch
>> (in preparation for a 1.0.1 branch).

I'm not going to go that far. As long as Marc signed up for cleaning  
up the trunk after the release is done, I'm fine.

>> If we determine that an RC5 is
>
> Note that we don't have a 1.0 branch; I believe that the consensus on
> that other thread was that the release branch should either directly
> become the maintenance branch, or the release branch should be copied
> to the maintenance branch.

What matters to me is the before-condition and the after-condition,  
which I've documented in the other branch (or tag) of this  
discussion. How the branch and tag are created are serving  
suggestions. But in order to encourage new release managers to  
volunteer, we should document what their rights, responsibilities,  
and best practices are.

I'll point out again that the difference between branch and tag is  
-0- to svn. It's only we who assign semantic value to a branch (which  
I mean to be a continuing code line) and a tag (which I consider to  
be a done finished complete point-in-time with a special name).

> In any event, it is pretty critical that
> the release branch contents make their way into the maintenance
> branch, and not via trunk.

Absolutely.

> Given that this is the case, it seems like
> putting minor fixes directly into the 1.0.0 branch is far and away the
> most expedient thing to do, since it means that in the case of a
> failed RC, the RM only needs to re-sync to the 1.0.0 branch. In the
> alternate case, the RM must do a patch-collection phase as well.

Again, "the RM only needs to do" stuff after the release is approved  
seems to unduly burden the RM.
>
> Additionally, FWIW, the concept of designating one person as the only
> person qualified to decide if a change should be in the release is
> directly counter to the release voting process.

Not agree.

> The RM may make a
> unilateral decision during the release-construction phase, but anyone
> can then vote against the release if the RM decides to exclude some
> bit of code from the release.

Not really. A release cannot be vetoed. If anyone has an issue with  
an excluded bit of code that the RM doesn't want to put in, a full  
vote is required.

> IOW, the voting process already provides
> a fail-safe to prevent worst-case scenarios; it seems like the rest of
> the process should be designed for expediency.

There are some cases where autocracy makes sense. I believe this is  
one of them. Long Live Release Manager.

By the way, nice job on the release, Marc. You've earned your  
vacation/honeymoon.

Craig

>
> -Patrick
>
> On 8/27/07, Kevin Sutter <kwsutter@gmail.com> wrote:
>> Patrick's premise of merging from the 1.0.0 branch back into trunk  
>> really
>> threw me...
>>
>> On 8/26/07, Patrick Linskey <plinskey@gmail.com> wrote:
>>>
>>>
>>> (In the case of the 1.0.0 line, I've been committing things to it
>>> directly and not to trunk, on the assumption that we'll be doing a
>>> bulk merge from 1.0.0 to trunk once we're done with the release.)
>>
>>
>> I must really be missing something here, but why would we merge  
>> the contents
>> of the 1.0.0 branch back into trunk after the release has been  
>> approved?  In
>> my mind, once a given x.y.z release has been approved, there are  
>> no more
>> code updates in this branch.  So, any updates done to this branch  
>> after the
>> approved svn revision are basically "lost".  They don't apply to  
>> any release
>> unless manually moved to the appropriate release.  I think I'm in  
>> agreement
>> with Craig here that once a RM has declared a given x.y.z release,  
>> then s/he
>> has complete control over what goes in or stays out.  We shouldn't be
>> putting more code against this branch.
>>
>> Here's how I thought the process should have worked...  When we  
>> determined
>> that we wanted to do the 1.0.0 release, we create the 1.0 branch like
>> Patrick proposed a few notes back.  Off of this branch, Marc would  
>> create
>> the 1.0.0 branch.  We've had a few RC's with fixes getting  
>> integrated into
>> the 1.0.0 branch.  Eventually, we declared RC4 on the 1.0.0 branch  
>> and
>> hopefully this one will pass.  But, each time Marc proposed a  
>> 1.0.0 build
>> for a vote, that branch was frozen and under his control as to  
>> what went in
>> and what stayed out.  Since Marc didn't explicitly request  
>> Patrick's changes
>> into the 1.0.0 branch (and thus force a respin), then Patrick's  
>> changes
>> should not have been put into the 1.0.0 branch.  At a minimum,  
>> Patrick's
>> changes should have been put into trunk and optionally into the  
>> 1.0 branch
>> (in preparation for a 1.0.1 branch).  If we determine that an RC5 is
>> required, then Marc may want Patrick's changes put into the 1.0.0  
>> branch for
>> a respin.  Then again, he may not want to introduce any more churn  
>> and only
>> take the specific change that is forcing him to do a respin.   
>> (Note that I
>> am only using Patrick's change as an example to discuss, not  
>> attempting to
>> associate any type of complexity with it.)
>>
>> As an aside, trunk has become 1.1.0-SNAPSHOT until we determine we  
>> need a
>> 1.1 branch.
>>
>> Granted, some of this discussion is with 20/20 hindsight.  But, we  
>> need to
>> figure out our approach and be consistent from the start.  All of  
>> us have
>> had various experiences with creating and maintaining branches and
>> releases.  We just need to find a common ground for us to live with.
>>
>> Thanks,
>> Kevin
>>
>> -Patrick
>>>
>>> On 8/26/07, Craig L Russell <Craig.Russell@sun.com> wrote:
>>>>
>>>> On Aug 26, 2007, at 5:51 PM, plinskey@gmail.com wrote:
>>>>
>>>>> Why? It seems like if the commiters are responsible about only
>>>>> committing things that would belong on the maintenance branch
>>>>> eventually anyways, then that's good enough.
>>>>
>>>> We're talking about a process here. The RM is trying to cut a  
>>>> release
>>>> and if developers are continuing to check code into all possible  
>>>> code
>>>> lines I smell disaster. IMHO the RM should be able to play in a
>>>> stable sandbox.
>>>>
>>>> Let's play this out with specifics. Let's say that the 1.0  
>>>> branch is
>>>> open to checkins while RM finalizes the release. At some point RM
>>>> tags 1.0.0 before building the release candidate. The 1.0 branch
>>>> continues to have checkins that developers have decided belong in
>>>> both trunk and maintenance. The vote fails. The RM merges the 1.0
>>>> branch into the 1.0.0 tag, picking up the checked in bug fixes that
>>>> developers thought would be good. Testing shows one of the bug  
>>>> fixes
>>>> is bad and the release has to be respun.
>>>>
>>>> A better process would be to give the RM control over the 1.0  
>>>> branch.
>>>> Nothing gets checked in unless RM approves it. RCs are cut from the
>>>> living end of 1.0 branch and voted on. At some point the vote
>>>> succeeds and tag 1.0.0 is created. The 1.0 branch is opened  
>>>> again to
>>>> developers, although I don't think many developers will bother
>>>> checking anything into it. Once someone wants to make a maintenance
>>>> release, RM goes through all the checkins from trunk to decide  
>>>> which
>>>> can safely be applied to the maintenance release and merges  
>>>> those in.
>>>>
>>>> I'm still curious about why developers would bother checking code
>>>> into both trunk and all the active maintenance branches. Do we want
>>>> developers to concern themselves with all the branches that the fix
>>>> applies to and run all their tests on each branch? Or do we have  
>>>> the
>>>> RM responsible for a maintenance branch review the patches that  
>>>> they
>>>> think are relevant?
>>>>
>>>> Craig
>>>>
>>>>> The benefit is that if
>>>>> the release does need to be rebuilt, then the additional tweaks
>>>>> automagically get picked up.
>>>>>
>>>>> Bear in mind that the RM can tag the revision used to make the RC.
>>>>>
>>>>> -Patrick
>>>>>
>>>>> On 8/26/07, Craig L Russell <Craig.Russell@sun.com> wrote:
>>>>>> Hi Kevin,
>>>>>>
>>>>>> On Aug 24, 2007, at 5:37 AM, Kevin Sutter wrote:
>>>>>>
>>>>>>> Patrick and Marc,
>>>>>>> Help me understand our process here.  This commit is similar
 
>>>>>>> to the
>>>>>>> one I
>>>>>>> did the other evening.  It was committed into the 1.0.0 branch
>>>>>>> after the 4th
>>>>>>> re-spin and [VOTE] was posted.  So, does this require yet  
>>>>>>> another
>>>>>>> respin?
>>>>>>
>>>>>> This is a problem. IMHO, the release manager should be the only
>>>>>> person deciding what gets checked into a branch that is going  
>>>>>> to be
>>>>>> released. If Marc ok'd the checkin after the release was  
>>>>>> voted, then
>>>>>> he should be prepared to respin the release. If he didn't ok the
>>>>>> checkin, then it should not have been checked in.
>>>>>>
>>>>>>> If not, then what happens to these changes?  The [VOTE] would
 
>>>>>>> not
>>>>>>> include
>>>>>>> these changes.  So, would these changes automatically become
 
>>>>>>> part
>>>>>>> of the
>>>>>>> 1.0.1 snapshot release?
>>>>>>
>>>>>> How I think this should be managed is the release manager for  
>>>>>> 1.0.0
>>>>>> should be approving all checkins into the branch. (Whether the  
>>>>>> branch
>>>>>> is named 1.0.0 or 1.0 isn't relevant to this discussion). Once  
>>>>>> a VOTE
>>>>>> is called, no further checkins should be allowed until the  
>>>>>> VOTE is
>>>>>> complete or rescinded. If rescinded, other checkins can happen  
>>>>>> at the
>>>>>> discretion of the release manager. Once complete, the branch  
>>>>>> should
>>>>>> be tagged and the contents reset to be a SNAPSHOT of the next  
>>>>>> release
>>>>>> on the branch. In this case, 1.0.1-SNAPSHOT.
>>>>>>
>>>>>> Craig
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Kevin
>>>>>>>
>>>>>>> On 8/24/07, pcl@apache.org <pcl@apache.org> wrote:
>>>>>>>>
>>>>>>>> Author: pcl
>>>>>>>> Date: Thu Aug 23 22:27:43 2007
>>>>>>>> New Revision: 569253
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=569253&view=rev
>>>>>>>> Log:
>>>>>>>> Minor logging / exception handling improvements
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>>
>>>>>>>>     openjpa/branches/1.0.0/openjpa-kernel/src/main/java/org/

>>>>>>>> apache/
>>>>>>>> openjpa/enhance/PCEnhancer.java
>>>>>>>>     openjpa/branches/1.0.0/openjpa-kernel/src/main/resources/

>>>>>>>> org/
>>>>>>>> apache/openjpa/enhance/localizer.properties
>>>>>>>>
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> openjpa/branches/1.0.0/openjpa-kernel/src/main/java/org/apache/
>>>>>>>> openjpa/enhance/PCEnhancer.java
>>>>>>>> URL:
>>>>>>>> http://svn.apache.org/viewvc/openjpa/branches/1.0.0/openjpa-

>>>>>>>> kernel/
>>>>>>>> src/main/java/org/apache/openjpa/enhance/PCEnhancer.java?
>>>>>>>> rev=569253&r1=569252&r2=569253&view=diff
>>>>>>>> ===============================================================

>>>>>>>> ====
>>>>>>>> ==
>>>>>>>> =========
>>>>>>>>
>>>>>>>> ---
>>>>>>>> openjpa/branches/1.0.0/openjpa-kernel/src/main/java/org/apache/
>>>>>>>> openjpa/enhance/PCEnhancer.java
>>>>>>>> (original)
>>>>>>>> +++
>>>>>>>> openjpa/branches/1.0.0/openjpa-kernel/src/main/java/org/apache/
>>>>>>>> openjpa/enhance/PCEnhancer.java
>>>>>>>> Thu Aug 23 22:27:43 2007
>>>>>>>> @@ -467,7 +467,8 @@
>>>>>>>>          } catch (OpenJPAException ke) {
>>>>>>>>              throw ke;
>>>>>>>>          } catch (Exception e) {
>>>>>>>> -            throw new GeneralException(e);
>>>>>>>> +            throw new GeneralException(_loc.get("enhance-

>>>>>>>> error",
>>>>>>>> +                _managedType.getType().getName(), e.getMessage
>>>>>>>> ()), e);
>>>>>>>>          }
>>>>>>>>      }
>>>>>>>>
>>>>>>>> @@ -2736,7 +2737,10 @@
>>>>>>>>              } catch (Throwable t) {
>>>>>>>>                  // last-chance catch for bug #283 (which
can
>>>>>>>> happen
>>>>>>>>                  // in a variety of ClassLoading environments)
>>>>>>>> -                _log.warn(_loc.get("enhance-uid-access",
>>>>>>>> _meta), t);
>>>>>>>> +                if (_log.isTraceEnabled())
>>>>>>>> +                    _log.warn(_loc.get("enhance-uid-access",
>>>>>>>> _meta), t);
>>>>>>>> +                else
>>>>>>>> +                    _log.warn(_loc.get("enhance-uid-access",
>>>>>>>> _meta));
>>>>>>>>              }
>>>>>>>>
>>>>>>>>              // if we couldn't access the serialVersionUID,
we
>>>>>>>> will have
>>>>>>>> to
>>>>>>>> @@ -3672,10 +3676,13 @@
>>>>>>>>       * attribute name for the backing field <code>name</code>.
>>>>>>>>       */
>>>>>>>>      private String fromBackingFieldName(String name) {
>>>>>>>> -        if (_meta.getAccessType() ==  
>>>>>>>> ClassMetaData.ACCESS_PROPERTY
>>>>>>>> +        // meta is null when doing persistence-aware  
>>>>>>>> enhancement
>>>>>>>> +        if (_meta != null
>>>>>>>> +            && _meta.getAccessType() ==
>>>>>>>> ClassMetaData.ACCESS_PROPERTY
>>>>>>>>              && _fieldsToAttrs.containsKey(name))
>>>>>>>> -            name = (String) _fieldsToAttrs.get(name);
>>>>>>>> -        return name;
>>>>>>>> +            return (String) _fieldsToAttrs.get(name);
>>>>>>>> +        else
>>>>>>>> +            return name;
>>>>>>>>      }
>>>>>>>>
>>>>>>>>      /**
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> openjpa/branches/1.0.0/openjpa-kernel/src/main/resources/org/
>>>>>>>> apache/openjpa/enhance/localizer.properties
>>>>>>>> URL:
>>>>>>>> http://svn.apache.org/viewvc/openjpa/branches/1.0.0/openjpa-

>>>>>>>> kernel/
>>>>>>>> src/main/resources/org/apache/openjpa/enhance/ 
>>>>>>>> localizer.properties?
>>>>>>>> rev=569253&r1=569252&r2=569253&view=diff
>>>>>>>> ===============================================================

>>>>>>>> ====
>>>>>>>> ==
>>>>>>>> =========
>>>>>>>>
>>>>>>>> ---
>>>>>>>> openjpa/branches/1.0.0/openjpa-kernel/src/main/resources/org/
>>>>>>>> apache/openjpa/enhance/localizer.properties
>>>>>>>> (original)
>>>>>>>> +++
>>>>>>>> openjpa/branches/1.0.0/openjpa-kernel/src/main/resources/org/
>>>>>>>> apache/openjpa/enhance/localizer.properties
>>>>>>>> Thu Aug 23 22:27:43 2007
>>>>>>>> @@ -197,4 +197,5 @@
>>>>>>>> no-accessor: Could not find method called {0} in type {1}.
>>>>>>>> unspecified-unenhanced-types: One or more of the types in
 
>>>>>>>> {0} have
>>>>>>>> relations \
>>>>>>>>      to other unenhanced types that were not specified. These
>>>>>>>> unspecified
>>>>>>>> types \
>>>>>>>> -    are: {1}
>>>>>>>> \ No newline at end of file
>>>>>>>> +    are: {1}
>>>>>>>> +enhance-error: An error occurred while enhancing {0}.  
>>>>>>>> Exception
>>>>>>>> message:
>>>>>>>> {1}
>>>>>>>> \ No newline at end of file
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>> Craig Russell
>>>>>> Architect, Sun Java Enterprise System http://java.sun.com/ 
>>>>>> products/
>>>>>> jdo
>>>>>> 408 276-5638 mailto:Craig.Russell@sun.com
>>>>>> P.S. A good JDO? O, Gasp!
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Patrick Linskey
>>>>> 202 669 5907
>>>>
>>>> Craig Russell
>>>> Architect, Sun Java Enterprise System http://java.sun.com/ 
>>>> products/jdo
>>>> 408 276-5638 mailto:Craig.Russell@sun.com
>>>> P.S. A good JDO? O, Gasp!
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Patrick Linskey
>>> 202 669 5907
>>>
>>
>
>
>
> -- 
> Patrick Linskey
> 202 669 5907

Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell@sun.com
P.S. A good JDO? O, Gasp!


Mime
View raw message