Re: [DISCUSS] Adjustments to the commit process

[Stephen Moist]

In that case Alex, could we just have a minimum of 2 people +1 it before you 
can commit it?

> On Feb 23, 2018, at 8:52 AM, Kalyan Kumar Kalvagadda  
> wrote:
> 
> Issue that you are talking about can be addressed by putting some
> additional guide lines in place.
> That way, as a process person who submits the patch should perform the same
> 'sanity check' before committing.
> Having another person responsible for sanity and commit complicates things.
> 
> -Kalyan
> 
> On Thu, Feb 22, 2018 at 8:25 PM, Alexander Kolbasov 
> wrote:
> 
>> For assigning committers I think this may be a simple informal request -
>> for example to one of the reviewers or to someone else to volunteer. It may
>> delay commits a bit indeed, but I don't think it will be a problem.
>> 
>> The problem I am trying to address is the quality of the review process.
>> Suppose we have some change C for which Alice have some comments and Bob
>> have some and eventually Alice says Ship it and it isn't clear whether Bob
>> is Ok with the change or not, but since ALice is the committer, the author
>> of the patch thinks that it is ok to submit it right away. That's where a
>> 'sanity check' person would be useful.
>> 
>> 
>> On Thu, Feb 22, 2018 at 3:49 PM, Sergio Pena 
>> wrote:
>> 
>>> How is this committer going to be assigned?
>>> This might lead to some complications if the committer assigned leave for
>>> vacations afterward and the community is not notified. It will end up
>>> delaying the commits and the author (being a committer) won't be able to
>>> commit the patch due to this process. What are we trying to solve with
>>> this?
>>> 
>>> Btw, I've seen in other projects that some committers usually wait 1 or 2
>>> days to commit a patch after a +1 has been done on it. This is to allow
>>> other reviewers to disagree with the +1 and give more feedback before
>>> committing the patch. Would this help?
>>> 
>>> - Sergio
>>> 
>>> On Thu, Feb 22, 2018 at 1:29 PM, Stephen Moist 
>> wrote:
>>> 
 Sounds reasonable to me as long as they can get someone to do the
>> commit
 in a reasonable timeframe.  I wouldn’t want to have to wait days for it
>>> to
 get in after it has been properly reviewed.
 
> On Feb 22, 2018, at 12:22 PM, Alexander Kolbasov >> 
 wrote:
> 
> Hello everyone,
> 
> I would like to propose an adjustment to the commit process in Sentry
> project. The idea is to require that commit should not be done by the
> person providing the change but by some other committer. This
>>> committer's
> responsibility is to ensure that all code review concerns were
>>> addressed
 in
> one way or another and to do a final sanity check. This committer can
>>> be
> one of the reviewers or someone who didn't review the code.
> 
> What do you think?
> 
> - Alex
 
 
>>> 
>> 

Re: [DISCUSS] Adjustments to the commit process

[Kalyan Kumar Kalvagadda]

Issue that you are talking about can be addressed by putting some
additional guide lines in place.
That way, as a process person who submits the patch should perform the same
'sanity check' before committing.
Having another person responsible for sanity and commit complicates things.

-Kalyan

On Thu, Feb 22, 2018 at 8:25 PM, Alexander Kolbasov 
wrote:

> For assigning committers I think this may be a simple informal request -
> for example to one of the reviewers or to someone else to volunteer. It may
> delay commits a bit indeed, but I don't think it will be a problem.
>
> The problem I am trying to address is the quality of the review process.
> Suppose we have some change C for which Alice have some comments and Bob
> have some and eventually Alice says Ship it and it isn't clear whether Bob
> is Ok with the change or not, but since ALice is the committer, the author
> of the patch thinks that it is ok to submit it right away. That's where a
> 'sanity check' person would be useful.
>
>
> On Thu, Feb 22, 2018 at 3:49 PM, Sergio Pena 
> wrote:
>
> > How is this committer going to be assigned?
> > This might lead to some complications if the committer assigned leave for
> > vacations afterward and the community is not notified. It will end up
> > delaying the commits and the author (being a committer) won't be able to
> > commit the patch due to this process. What are we trying to solve with
> > this?
> >
> > Btw, I've seen in other projects that some committers usually wait 1 or 2
> > days to commit a patch after a +1 has been done on it. This is to allow
> > other reviewers to disagree with the +1 and give more feedback before
> > committing the patch. Would this help?
> >
> > - Sergio
> >
> > On Thu, Feb 22, 2018 at 1:29 PM, Stephen Moist 
> wrote:
> >
> > > Sounds reasonable to me as long as they can get someone to do the
> commit
> > > in a reasonable timeframe.  I wouldn’t want to have to wait days for it
> > to
> > > get in after it has been properly reviewed.
> > >
> > > > On Feb 22, 2018, at 12:22 PM, Alexander Kolbasov  >
> > > wrote:
> > > >
> > > > Hello everyone,
> > > >
> > > > I would like to propose an adjustment to the commit process in Sentry
> > > > project. The idea is to require that commit should not be done by the
> > > > person providing the change but by some other committer. This
> > committer's
> > > > responsibility is to ensure that all code review concerns were
> > addressed
> > > in
> > > > one way or another and to do a final sanity check. This committer can
> > be
> > > > one of the reviewers or someone who didn't review the code.
> > > >
> > > > What do you think?
> > > >
> > > > - Alex
> > >
> > >
> >
>

Re: [DISCUSS] Adjustments to the commit process

[Alexander Kolbasov]

For assigning committers I think this may be a simple informal request -
for example to one of the reviewers or to someone else to volunteer. It may
delay commits a bit indeed, but I don't think it will be a problem.

The problem I am trying to address is the quality of the review process.
Suppose we have some change C for which Alice have some comments and Bob
have some and eventually Alice says Ship it and it isn't clear whether Bob
is Ok with the change or not, but since ALice is the committer, the author
of the patch thinks that it is ok to submit it right away. That's where a
'sanity check' person would be useful.


On Thu, Feb 22, 2018 at 3:49 PM, Sergio Pena 
wrote:

> How is this committer going to be assigned?
> This might lead to some complications if the committer assigned leave for
> vacations afterward and the community is not notified. It will end up
> delaying the commits and the author (being a committer) won't be able to
> commit the patch due to this process. What are we trying to solve with
> this?
>
> Btw, I've seen in other projects that some committers usually wait 1 or 2
> days to commit a patch after a +1 has been done on it. This is to allow
> other reviewers to disagree with the +1 and give more feedback before
> committing the patch. Would this help?
>
> - Sergio
>
> On Thu, Feb 22, 2018 at 1:29 PM, Stephen Moist  wrote:
>
> > Sounds reasonable to me as long as they can get someone to do the commit
> > in a reasonable timeframe.  I wouldn’t want to have to wait days for it
> to
> > get in after it has been properly reviewed.
> >
> > > On Feb 22, 2018, at 12:22 PM, Alexander Kolbasov 
> > wrote:
> > >
> > > Hello everyone,
> > >
> > > I would like to propose an adjustment to the commit process in Sentry
> > > project. The idea is to require that commit should not be done by the
> > > person providing the change but by some other committer. This
> committer's
> > > responsibility is to ensure that all code review concerns were
> addressed
> > in
> > > one way or another and to do a final sanity check. This committer can
> be
> > > one of the reviewers or someone who didn't review the code.
> > >
> > > What do you think?
> > >
> > > - Alex
> >
> >
>

Re: [DISCUSS] Adjustments to the commit process

[Sergio Pena]

How is this committer going to be assigned?
This might lead to some complications if the committer assigned leave for
vacations afterward and the community is not notified. It will end up
delaying the commits and the author (being a committer) won't be able to
commit the patch due to this process. What are we trying to solve with this?

Btw, I've seen in other projects that some committers usually wait 1 or 2
days to commit a patch after a +1 has been done on it. This is to allow
other reviewers to disagree with the +1 and give more feedback before
committing the patch. Would this help?

- Sergio

On Thu, Feb 22, 2018 at 1:29 PM, Stephen Moist  wrote:

> Sounds reasonable to me as long as they can get someone to do the commit
> in a reasonable timeframe.  I wouldn’t want to have to wait days for it to
> get in after it has been properly reviewed.
>
> > On Feb 22, 2018, at 12:22 PM, Alexander Kolbasov 
> wrote:
> >
> > Hello everyone,
> >
> > I would like to propose an adjustment to the commit process in Sentry
> > project. The idea is to require that commit should not be done by the
> > person providing the change but by some other committer. This committer's
> > responsibility is to ensure that all code review concerns were addressed
> in
> > one way or another and to do a final sanity check. This committer can be
> > one of the reviewers or someone who didn't review the code.
> >
> > What do you think?
> >
> > - Alex
>
>

Re: [DISCUSS] Adjustments to the commit process

[Stephen Moist]

Sounds reasonable to me as long as they can get someone to do the commit in a 
reasonable timeframe.  I wouldn’t want to have to wait days for it to get in 
after it has been properly reviewed. 

> On Feb 22, 2018, at 12:22 PM, Alexander Kolbasov  wrote:
> 
> Hello everyone,
> 
> I would like to propose an adjustment to the commit process in Sentry
> project. The idea is to require that commit should not be done by the
> person providing the change but by some other committer. This committer's
> responsibility is to ensure that all code review concerns were addressed in
> one way or another and to do a final sanity check. This committer can be
> one of the reviewers or someone who didn't review the code.
> 
> What do you think?
> 
> - Alex