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 04:29:32 GMT
In  the case of this checkin, I checked it in based on some debugging
that I was doing into potential problems with the 1.0.0 release. So, I
was evaulating the 1.0.0 release, and made some minor fixes along the
way. (In this case, better error messages from the enhancer.)

I did not check these changes into trunk. I only checked them into
1.0.0, since I know that the 1.0.0 line will eventually be merged down
to trunk, and so checkins I make there will make their way to trunk in
their own good time.

This change was something that I believe we would want to put into
1.0.0 if we needed to respin, but is not something worth restarting
the 1.0.0 process for. So, I checked it into the branch that a
hypothetical next cut would come from; if we needed another, it would
get into the next build; if not, it would end up in both 1.0.1 (since
my checkin was on the 1.0.0 branch; assuming that we do an svn mv of
1.0.0 to 1.0.1) and trunk (when the 1.0.0 => trunk merge happens).

IOW, this was a hardening change for the 1.0.0 process, but one that
was too trivial to require a new release (in my opinion).

I think that your process does a good job of isolating the release
management procedure, but it would have made things more complex for
both Marc and I. I do not think that the isolation concern is a
problem that really needs to be solved. I think that to the extent
that we can informally make things work out properly, we should try to
use a more flexible process.

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

In my opinion, I think we should let developers check things in
wherever they want to. I think that most development work should
happen in trunk unless we have a reason for it to happen elsewhere.
So, for example, if a developer has a fix that I think needs to be in
a patch release, I think that it should be up to that developer to put
the fix into the appropriate release branch. We still haven't really
discussed merging that much in that other thread; if we go with a
model in which it is not the RM's responsibility to do a post-release
merge to trunk, then we should require developers to put work into
trunk as well as whatever release branch they deem fit, as
appropriate.

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

-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

Mime
View raw message