Review/ Commit policy

[Edward Capriolo <ed...@gmail.com>]

All,

Does the incubator have a policy on review + commit?

I have worked on other top level projects. Typically they have multiple
people on the project and have critical mass to make sure things get
reviewed.

Here not many are experienced with the code-base and may make other
hesitant to do a review.

I think it makes sense that if a Committer on Gossip does not get a review
in 1 day a self merge is ok.

If this not acceptable I would like to established a pool of people that
are available for reviews and something like a soft SLA between us so we
know the worse case for action.

In particular I am going to want to move things forward and I might
outstrip others availability.

Re: Review/ Commit policy

[Josh Elser <jo...@gmail.com>]

Edward Capriolo wrote:
> All,
>
> Does the incubator have a policy on review + commit?
>
> I have worked on other top level projects. Typically they have multiple
> people on the project and have critical mass to make sure things get
> reviewed.
>
> Here not many are experienced with the code-base and may make other
> hesitant to do a review.
>
> I think it makes sense that if a Committer on Gossip does not get a review
> in 1 day a self merge is ok.

Sean gave a nice long-form answer. My tl;dr opinion is "find something 
that works for you". I've read many an argument on ctr and rtc; 
ultimately, I think it depends on your community. Pick something, 
document it, and see how it works out. :)

> If this not acceptable I would like to established a pool of people that
> are available for reviews and something like a soft SLA between us so we
> know the worse case for action.

I'm kind of curious now if you would actually need any sort of IP 
clearance just for review feedback. I believe as long as you have 
straightforward rules that define who is "trusted", it would be fine.

> In particular I am going to want to move things forward and I might
> outstrip others availability.
>

Re: Review/ Commit policy

[Edward Capriolo <ed...@gmail.com>]

In general I am cool with commit then review. What juat happened to me is I
have had a patch up for a couple of days and something else just came in. I
will review and merge the something else, and I will have to rebase my bits.

Also I do not have a feel for who is in the review pool and what the turn
around will be.

On Saturday, June 4, 2016, Sean Busbey <bu...@apache.org> wrote:

> The incubator has no policy on commit policies for individual podlings. The
> general assumption is most will work it out for themselves. This is a great
> discussion to have now while Gossip is still early days.
>
> Just to state biases, I personally prefer Review-Then-Commit. I think it
> helps communities in a couple of ways.
>
> For one, it increases the odds that multiple people understand parts of the
> codebase. In particular, when a project gets thin on folks who are
> reviewing a given section it creates a very powerful incentive to recruit
> more people in that area.
>
> More importantly, I think it helps to give new comers a sense of fairness
> in participation. Essentially, they get to see that they aren't the only
> ones going through this additional work and they can see that we all learn
> from each other as the project goes.
>
> If you're concerned about available review bandwidth, I can think of a
> couple of mitigation strategies that I've seen work well in new / small
> communities.
>
> The biggest one has been to make non-committer reviews binding. This is a
> great way to show that the community values reviews as contributions. I've
> seen several communities that state that reviews are a way to "act like a
> committer" but that different from the level of investment required to say
> you'll have impact on the project's progress if you show up doing reviews
> day one.
>
> The other approach is more a hedge. It's essentially "branch then merge"
> for periods so that a contributor who's coding faster than reviews happen
> can batch things e.g. weekly.
>
> Related to this discussion (and a big part of my preference for RTC with
> binding non-committer reviews) is the matter of tracking review work. One
> thing that's worked really well on Apache Yetus is to make sure commits
> that go in have:
>
> 1) the git author set to the contributor who wrote the patch
>
> 2) a signed-off-by annotation for each person who provided a review (again,
> wether they're a committer or not).
>
> This combination makes it much much easier for interested parties to get a
> sense of the folks involved in these two important ways of contributing to
> the project.
> On Jun 4, 2016 10:51, "Edward Capriolo" <edlinuxguru@gmail.com
> <javascript:;>> wrote:
>
> > All,
> >
> > Does the incubator have a policy on review + commit?
> >
> > I have worked on other top level projects. Typically they have multiple
> > people on the project and have critical mass to make sure things get
> > reviewed.
> >
> > Here not many are experienced with the code-base and may make other
> > hesitant to do a review.
> >
> > I think it makes sense that if a Committer on Gossip does not get a
> review
> > in 1 day a self merge is ok.
> >
> > If this not acceptable I would like to established a pool of people that
> > are available for reviews and something like a soft SLA between us so we
> > know the worse case for action.
> >
> > In particular I am going to want to move things forward and I might
> > outstrip others availability.
> >
>


-- 
Sorry this was sent from mobile. Will do less grammar and spell check than
usual.

Re: Review/ Commit policy

[Sean Busbey <bu...@apache.org>]

The incubator has no policy on commit policies for individual podlings. The
general assumption is most will work it out for themselves. This is a great
discussion to have now while Gossip is still early days.

Just to state biases, I personally prefer Review-Then-Commit. I think it
helps communities in a couple of ways.

For one, it increases the odds that multiple people understand parts of the
codebase. In particular, when a project gets thin on folks who are
reviewing a given section it creates a very powerful incentive to recruit
more people in that area.

More importantly, I think it helps to give new comers a sense of fairness
in participation. Essentially, they get to see that they aren't the only
ones going through this additional work and they can see that we all learn
from each other as the project goes.

If you're concerned about available review bandwidth, I can think of a
couple of mitigation strategies that I've seen work well in new / small
communities.

The biggest one has been to make non-committer reviews binding. This is a
great way to show that the community values reviews as contributions. I've
seen several communities that state that reviews are a way to "act like a
committer" but that different from the level of investment required to say
you'll have impact on the project's progress if you show up doing reviews
day one.

The other approach is more a hedge. It's essentially "branch then merge"
for periods so that a contributor who's coding faster than reviews happen
can batch things e.g. weekly.

Related to this discussion (and a big part of my preference for RTC with
binding non-committer reviews) is the matter of tracking review work. One
thing that's worked really well on Apache Yetus is to make sure commits
that go in have:

1) the git author set to the contributor who wrote the patch

2) a signed-off-by annotation for each person who provided a review (again,
wether they're a committer or not).

This combination makes it much much easier for interested parties to get a
sense of the folks involved in these two important ways of contributing to
the project.
On Jun 4, 2016 10:51, "Edward Capriolo" <ed...@gmail.com> wrote:

> All,
>
> Does the incubator have a policy on review + commit?
>
> I have worked on other top level projects. Typically they have multiple
> people on the project and have critical mass to make sure things get
> reviewed.
>
> Here not many are experienced with the code-base and may make other
> hesitant to do a review.
>
> I think it makes sense that if a Committer on Gossip does not get a review
> in 1 day a self merge is ok.
>
> If this not acceptable I would like to established a pool of people that
> are available for reviews and something like a soft SLA between us so we
> know the worse case for action.
>
> In particular I am going to want to move things forward and I might
> outstrip others availability.
>