From dev-return-32191-archive-asf-public=cust-asf.ponee.io@geode.apache.org Mon Oct 21 17:36:36 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 3555E180626 for ; Mon, 21 Oct 2019 19:36:36 +0200 (CEST) Received: (qmail 93232 invoked by uid 500); 21 Oct 2019 17:36:35 -0000 Mailing-List: contact dev-help@geode.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@geode.apache.org Delivered-To: mailing list dev@geode.apache.org Received: (qmail 93217 invoked by uid 99); 21 Oct 2019 17:36:35 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 21 Oct 2019 17:36:35 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id A232BC22A0 for ; Mon, 21 Oct 2019 17:36:34 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -0.499 X-Spam-Level: X-Spam-Status: No, score=-0.499 tagged_above=-999 required=6.31 tests=[HTML_MESSAGE=0.2, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Received: from mx1-ec2-va.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id ZbSj1da8OmPQ for ; Mon, 21 Oct 2019 17:36:31 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=148.163.150.38; helo=mx0a-00296801.pphosted.com; envelope-from=rhoughton@pivotal.io; receiver= Received: from mx0a-00296801.pphosted.com (mx0a-00296801.pphosted.com [148.163.150.38]) by mx1-ec2-va.apache.org (ASF Mail Server at mx1-ec2-va.apache.org) with ESMTPS id 515DFBCBA7 for ; Mon, 21 Oct 2019 17:30:18 +0000 (UTC) Received: from pps.filterd (m0114581.ppops.net [127.0.0.1]) by mx0a-00296801.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id x9LHPTph004815 for ; Mon, 21 Oct 2019 17:30:17 GMT Received: from mail-lf1-f71.google.com (mail-lf1-f71.google.com [209.85.167.71]) by mx0a-00296801.pphosted.com with ESMTP id 2vqs3h1fhh-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Mon, 21 Oct 2019 17:30:17 +0000 Received: by mail-lf1-f71.google.com with SMTP id 23so2658123lft.15 for ; Mon, 21 Oct 2019 10:30:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=t3EW56teSkN6xLNJdJlp6c+yEM45xnZNgvh4vTR4ABs=; b=F3tMfzjnNzdrep5epSB4gc7A5ESM+ChECmp3AAVeOA+/hqBm4KSfZerZWSHfrD0Xc1 yy66jvU7yYZBqfDHyWRQ8ICXJghvWbt4VCIACrM55N6X6UnE11oOCQgYHPhFMXWM3WiO nB4X0rUcxWJI/xdhj+3kk376cz1F1mUWk6PerUyoPj0JfczWvL0L0GjJdtEr8gXVIzUG Ej3Q7nBwP7/CZiWzVLOT6WiYad7Fz0bACqCk5I5OyiM03+kTtAzuC7fs7/yll8tTwSgy ervT5ijBmjMwYkcVo6Kc0fPlsLsPQKplvf8ZYAoNt0EPf6VSMCnfmeWD8WLzU9IFT9E/ cCZg== X-Gm-Message-State: APjAAAUasHW7H6bLDq+4seuV6H1cpXTKo/LOid+BhTXGa081CPNqnCz+ fQoUUIWXYTdcuChm7jicvKRdlU3GwbsP3xGoAu2xpAZzwbMzIRqetothFnjvSgDAdwvG2bNWymV 6VZjIDqqBYiLNBENLixPXZY+sg+o1ThC6hlbNyH5vZsNmXb5UKUrhLhA= X-Received: by 2002:ac2:484e:: with SMTP id 14mr9003392lfy.184.1571679014799; Mon, 21 Oct 2019 10:30:14 -0700 (PDT) X-Google-Smtp-Source: APXvYqxACG8aT46EIzEGtFhb8Q4MvyG/i3Qv0vok8hJM2ijoRowPpbncKKG0mESnTQfO3on9j/JK0SuVcJj1WfGXDGs= X-Received: by 2002:ac2:484e:: with SMTP id 14mr9003380lfy.184.1571679014480; Mon, 21 Oct 2019 10:30:14 -0700 (PDT) MIME-Version: 1.0 References: <4644E453-01C4-4761-808C-FF3CE3A6994A@pivotal.io> <7a637c15-1e32-d185-e49f-1b55a14c3663@pivotal.io> <592DC140-4820-4D47-BD5B-9FC4D7F0A111@pivotal.io> <211fa122-19f1-cd87-b0b1-48f05010d087@apache.com> In-Reply-To: <211fa122-19f1-cd87-b0b1-48f05010d087@apache.com> From: Robert Houghton Date: Mon, 21 Oct 2019 10:30:03 -0700 Message-ID: Subject: Re: [DISCUSS] Blocking merge button in PR To: dev@geode.apache.org, Udo Kohlmeyer Content-Type: multipart/alternative; boundary="0000000000005c097a05956f0940" X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.95,1.0.8 definitions=2019-10-21_04:2019-10-21,2019-10-21 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 phishscore=0 bulkscore=0 impostorscore=0 clxscore=1015 priorityscore=1501 suspectscore=0 lowpriorityscore=0 mlxscore=0 adultscore=0 malwarescore=0 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-1908290000 definitions=main-1910210166 --0000000000005c097a05956f0940 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable @Udo Kohlmeyer 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 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 wi= th > > 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 > 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 hav= e > 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 an= d > >> 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 > >> 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 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 peopl= e > >>>> 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 w= e > >>>> should require *at least one* official approval. > >>>> Cheers. > >>>> > >>> > >>> -- > >>> *Joris Melchior * > >>> CF Engineering > >>> Pivotal Toronto > >>> 416 877 5427 > >>> > >>> =E2=80=9CPrograms must be written for people to read, and only incide= ntally for > >>> machines to execute.=E2=80=9D =E2=80=93 *Hal Abelson* > >>> > >>> > --0000000000005c097a05956f0940--