incubator-odf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rob Weir <robw...@apache.org>
Subject Re: Review of changes
Date Fri, 02 Mar 2012 14:38:47 GMT
On Fri, Mar 2, 2012 at 6:15 AM, Svante Schubert
<svante.schubert@gmail.com> wrote:
> It seems we moved to the Apache default doing a first commit and review
> later in the main trunk, which leads to a more instable trunk. The
> approach to be able to deliver any time is much more desirable as we
> need as many people as possible working on the trunk
>

The fewer impediments we have to contributions, the more contributors
will be willing to work with us. A high hurdle to contributions will
dissuade contributors.

In any case, CTR applies to committers only.  Non-committer developers
still need to submit patches, and these are reviewed before
committing.

So rather than introducing a 3-day delay on every code change, which
can often turn into a much longer delay as we wait for a reviewer (we
are a small project), maybe we can do something like this:

Agree on a mandatory set of "desk checks" that committers must do
before checking in code.  For example, one set of rules might me:

1) Do an svn update to ensure that you have the current trunk code

2) Do a complete build of all changed components, as well as any
components that depend on the changed components.

3) Complete build means: mvn clean install -Ppedantic

4) Build must complete with no errors


That would work for the problems we've seen recently. Remember, we
have not seen subtle logic errors that would only be caught by an in
depth code review. We've just seen some simple breaks due to POM
changes.  They would have been prevented by the above steps.

> Therefore in general - unless fixing a red master - I would recommend to
> do a review on patches and allow people three days (similar to voting
> time) to review the changes. For larger patches a week might be more
> appropriate.

I don't agree, at least not in most cases. Maybe we switch to RTC when
we get close to release.  For example, after we declare a Release
Candidate we could switch to RTC.  But for most of the development
cycle we should be CRT.

> Especially I am worried about a fast removal of the DOC API as I fear
> that functionality get lost. For instance the template initialization
> routine (see OfficeMetaTest.updateTemplates()) would have gotten lost
> and in addition I realized it is no longer possible in the Simple API to
> remove a property (as creator) by providing NULL as parameter. This
> seems like a regression of functionality compared with the DOC API. We
> need to identify, discuss and fix those issues before removing DOC.
>

That is not a CTR versus RTC issue.  This is a question of whether to
make this change at all.   If a change is likely to be controversial
then it should be discussed on the list first.

-Rob

> Thanks,
> Svante

Mime
View raw message