@Robert Houghton,
Tbh honest, if we find that we have committers that are NOT following
procedure/protocol, then there is a PMC that this needs to be escalated to.
IF said committer is willfully acting in a negative manner towards the
project, I am fully supportive of removing the rights and privileges for
that committer, maybe from the project in totality.
But if this is merely an educational effort then that will be provided
as well.
But we all know that we can employ as many defensive mechanisms as we'd
like, but in the end we are only end up hurting the project. As all the
extra defensive red tape will eventually inhibit the project. That is
how all bureaucracy starts.. small innocent.. and eventually it becomes
this heavy beast that no-one can master.
--Udo
On 10/21/19 10:30 AM, Robert Houghton wrote:
@Udo Kohlmeyer <mailto:ukohlme...@pivotal.io> What, if anything, do
you propose we do to help keep our project building and running
cleanly that does not force punitive or coercive behavior on our
developers? "Naming names" or "shaming" are not popular choices, and
everyone on the comitters list *should* follow procedure, but doesn't.
What would you do?
On Mon, Oct 21, 2019 at 10:18 AM Udo Kohlmeyer <u...@apache.com
<mailto:u...@apache.com>> wrote:
I must agree with @Karen here..
All committers are trusted to do the right thing. One of those
things is
to commit (or not commit) PR's.
Now we are discussing disabling the button when tests are failing.
Why
stop there? Why not, that the submitter of the said commit does
not get
to merge their own PR?
Now, that of course is taking it to the extreme, that we don't
want (at
least I don't want to be THAT over prescriptive).. So why do we
want to
now limit when I can merge? It remains the committers
responsibility to
merge commits into the project, that are of the expected quality.
IF it
so happens that one, by accident, has merged a PR before it was
green,
revert it. All committers have the power to do so.
So from my perspective, a -1 on disabling the Merge button, just
because
someone was not careful in merging and without following our
protocol of
waiting for an "All green".
--Udo
On 10/21/19 10:11 AM, Ernest Burghardt wrote:
> +1 to enacting this immediately... just this weekend a PR was
merged with
> failures on all of the following:
> * concourse-ci/DistributedTestOpenJDK11 * Concourse CI build failure
> * concourse-ci/UnitTestOpenJDK11 * Concourse CI build failure
> * concourse-ci/UnitTestOpenJDK8 * Concourse CI build failure
> * concourse-ci/UpgradeTestOpenJDK11 * Concourse CI build failure
>
>
> Thanks,
> EB
>
> On Mon, Oct 21, 2019 at 10:01 AM Karen Miller
<kmil...@pivotal.io <mailto:kmil...@pivotal.io>> wrote:
>
>> I have (more than once) committed docs changes for typo fixes
without
>> review. I generally label the commits
>> with a bold "Commit then Review" message. But, I am bringing
this up as I
>> have purposely not followed what
>> looks to be a positively-received proposed policy, since I have
not gotten
>> reviews. If all feel that we need a
>> rule for everyone to follow (instead of a guideline that PRs
shall have at
>> least 1 review), I will follow the rule,
>> but I'm a -0 on the process. I get it, and I understand its
purpose and
>> intent, but I personally prefer to trust that each
>> comitter takes personal responsibility for the code they commit
WRT waiting
>> for tests and/or obtaining reviews.
>> Karen
>>
>>
>> On Mon, Oct 21, 2019 at 6:24 AM Joris Melchior
<jmelch...@pivotal.io <mailto:jmelch...@pivotal.io>>
>> wrote:
>>
>>> +1 to the revised approach. I think requiring at least one
review is
>>> important. More eyes make for better code.
>>>
>>> Cheers, Joris.
>>>
>>> On Mon, Oct 21, 2019 at 8:11 AM Ju@N <jujora...@gmail.com
<mailto:jujora...@gmail.com>> wrote:
>>>
>>>> +10 to Naba's proposal, it seems the right thing to do and
will help us
>>> to
>>>> prevent accidentally breaking *develop* while keeping focus
on people
>>>> instead of processes.
>>>> I'd add, however, that the *Merge Pull Request* button should
remain
>>>> disabled until *all CIs have finished*, and only enable it
once the
>>> *Build,
>>>> Unit, Stress Tests and LGTM are green *(that is, force the
committer to
>>>> wait at least until all CIs are done)*. *I also agree in that
that we
>>>> should require *at least one* official approval.
>>>> Cheers.
>>>>
>>>
>>> --
>>> *Joris Melchior *
>>> CF Engineering
>>> Pivotal Toronto
>>> 416 877 5427
>>>
>>> “Programs must be written for people to read, and only
incidentally for
>>> machines to execute.” – *Hal Abelson*
>>> <https://en.wikipedia.org/wiki/Hal_Abelson>
>>>
