openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Patrick Linskey" <plins...@gmail.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 Mon, 27 Aug 2007 23:09:49 GMT
> 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. 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?

> 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. 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.

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, whereas I feel it's more
appropriate to design a system that is easy to handle in the happy
case. 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.

> 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

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. In any event, it is pretty critical that
the release branch contents make their way into the maintenance
branch, and not via trunk. 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.

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. 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. 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.

-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

Mime
View raw message