From dev-return-3530-archive-asf-public=cust-asf.ponee.io@mxnet.incubator.apache.org Thu Jul 12 14:20:04 2018 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 [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 7117D180654 for ; Thu, 12 Jul 2018 14:20:03 +0200 (CEST) Received: (qmail 40491 invoked by uid 500); 12 Jul 2018 12:20:02 -0000 Mailing-List: contact dev-help@mxnet.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@mxnet.incubator.apache.org Delivered-To: mailing list dev@mxnet.incubator.apache.org Received: (qmail 40096 invoked by uid 99); 12 Jul 2018 12:20:01 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 12 Jul 2018 12:20:01 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id E2C5EC5DD1 for ; Thu, 12 Jul 2018 12:20:00 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.869 X-Spam-Level: ** X-Spam-Status: No, score=2.869 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_REPLY=1, HTML_MESSAGE=2, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001, T_DKIMWL_WL_MED=-0.01] autolearn=disabled Authentication-Results: spamd1-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id i-tp4vSQzRQy for ; Thu, 12 Jul 2018 12:19:59 +0000 (UTC) Received: from mail-yb0-f174.google.com (mail-yb0-f174.google.com [209.85.213.174]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id C1F1C5F366 for ; Thu, 12 Jul 2018 12:19:59 +0000 (UTC) Received: by mail-yb0-f174.google.com with SMTP id s8-v6so11309890ybe.8 for ; Thu, 12 Jul 2018 05:19:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=QwNUMnX8KNdKkGAWsXtET0f2XcSkA4S1sSc8m7i2jFQ=; b=RiBRgxEUcM0kDm07DaxZgxjOwkGtsKdNnMi/hCRg2HjzE45jGkfFF+ydKs7BKn/j3r 55eKz+MrVL47FUETxk2qZVEJT8s5XfEvWK+iL952Q6B0gWjMLOG2pS8RpoVgBAlt+9gr upI/abWWAl6h7Vo5DaIHVozaXZPzi+LetcpNaenb3GbiLu+xJFxQtYx9RJcxS9nPBqox oT/7gNYzedsNLUflbIL6Bwor0tMDxFPueF1wdSnZIrEYH049Zace/a85VszPAayZ1bg5 KNFUWCzM/dlioZ7R8vjybb8jCJytMBS/dY936Kq1s9d6ICPcUOetFA57zvMbKgLbvKhY jnVQ== 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=QwNUMnX8KNdKkGAWsXtET0f2XcSkA4S1sSc8m7i2jFQ=; b=NcCaTs0ynMJvMCg44NfrNxixVSlMXKqgpCyWj6sjqVyhwRCOHz+0mUwWr4Zd5wRcZy oD0A0FhjdwU09KExHHDjsyCgUijOXpXfMCsUYkWwo6Ffve2tCE3trJoxyBfV4cgfJTej oDfUqI+SmTzDdXMIz4lv0D9WmRFcNIebcL7uTwdlIfnAo+s+giCaptdy5rtJscuh56+t 8xFi6w/NkRiQj8SDg3a5B46DmhWEaCc1+awm4j8ONdiVmKmv427gkhMbcQPqbUMuRLX0 /GyFPhFIbkek5rkIZ48eJkunCLv62/KkZ098nRFVoZEzYBeTm+kd4Mwy90q7Q9G4K3QU 3mFA== X-Gm-Message-State: AOUpUlEMQ5AqhPkELzIlHzXovmUff4OvElSDrAaOyTeNan+3gRDUShjj c9XXqYPMBEi4K9TF5nrUfVLeyvnKoI0trTPFkZWG7g== X-Google-Smtp-Source: AAOMgpf0u7M0DiOM+PZyZtYRoqAHrcd4rwGUUNMSHxcuTjuEpuT8FWFm5Vg+Vp8O2mJr9CJ2BmmR4OvWthHEScOsfBs= X-Received: by 2002:a25:3886:: with SMTP id f128-v6mr949378yba.236.1531397992863; Thu, 12 Jul 2018 05:19:52 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Anton Chernov Date: Thu, 12 Jul 2018 14:19:41 +0200 Message-ID: Subject: Re: Squash/Merge PRs To: dev@mxnet.incubator.apache.org Content-Type: multipart/alternative; boundary="0000000000005fdb7d0570cc61d3" --0000000000005fdb7d0570cc61d3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Unfortunately there has been significant push back for small iterative PR's and as a result they have grown substantially involving multiple contributors, multiple commits, sometimes multiple branches to be worked on= . I fully agree and support all points that Jin raised: 1) Most contributions should be broken down into smallest possible PR's. 2) If a PR is small enough a single person can complete it. 3) If a majority of PR is done by someone, and there's some minor issue he/she needs help with it could be done in a follow up PR by anybody, including the reviewer. Best regards, Anton =D1=87=D1=82, 12 =D0=B8=D1=8E=D0=BB. 2018 =D0=B3. =D0=B2 10:11, Hao Jin : > +1 for Marco's proposal on using the co-author field. IMHO the usage of > co-author field should not be that often, consider: > 1) If a PR is so big that it needs multiple people to contribute a > substantial part of it, why can't that PR be broken down into smaller PRs > each submitted by single one of them? > 2) If a PR is small enough (for example, < 300 lines), why can't a single > person complete it? > 3) If a majority of PR is done by someone, and there's some minor issue > he/she needs help with(a small bug, incomplete doc), why can't that be do= ne > through code reviews? > From the above 3 situations we can see that collaborations on individual > PRs should not be quite often, but I do agree that it could still be > necessary when someone lacks the related expertise/knowledge on some > necessary part of that PR given that the required knowledge cannot be > picked up in a short period of time. > > I do agree that keeping the commit histories of PRs clean is very importa= nt > as it could be confusing when reviewing PRs, but that really depends on > personal preferences (For my case, I usually do "git commit --amend" on > trivial changes and get a new commit for bigger changes such as a > checkpoint of my whole PR). With growing the community and attracting mor= e > contributors as a high priority for our project, I would prefer that we d= o > not put even more burden on the contributors by asking them to manage and > squash the commits themselves just for the not-so-often cases of having > multiple contributors on a single PR. > Best regards, > Hao > > On Thu, Jul 12, 2018 at 12:35 AM, Marco de Abreu < > marco.g.abreu@googlemail.com.invalid> wrote: > > > Hi Naveen, > > > > I'm in favour of the squashing, considering the number of commits in so= me > > PRs and especially because of some people making commit messages a la > "fix" > > "fix" "fix" all the time. Additionally, it gets hard (not impossible, > just > > more inconvenient) to determine the atomic states of master - aka, whic= h > > commits are separate from each other. You should consider that > intermediary > > commits are unstable (fail CI) and thus it could be very hard to bisect > > failures in future - and the commit history gets cluttered. > > > > As alternative, I'd like to suggest the co-author field for these cases= . > > Further documentation is available at > > https://blog.github.com/2018-01-29-commit-together-with-co-authors/. > > > > I definitely agree with the second part. We should all lead by example > and > > maintain a high quality by keeping our commit messages clean and > > meaningful. When I receive an email notification that a new commit has > been > > added and it only contains "fix" as title, it's not that helpful and al= so > > it's hard to track the development of a PR overtime. E.g., why has > > something been changed? Was there maybe a bug that we didn't cover with > > tests but the author just hacked something to get it to work but the > > problem still lays somewhere? We won't know that way and it makes it > harder > > for us to review. > > > > Best regards, > > Marco > > > > > > Naveen Swamy schrieb am Do., 12. Juli 2018, 10:09: > > > > > Hi All, > > > > > > I am seeing that maintainers merge PRs into the repo, they are > squashing > > > the commits in the PR, which I understand and agree is to keep a sane > > > commit history, however this is causing problem when there are multip= le > > > contributors involved on a PR(by contributing to a fork of the repo) > this > > > effectively removes credit for multiple contributors involved and sho= ws > > all > > > code as authored by the contributor who created the PR. > > > > > > Can I request maintainers to not squash PRs if there are multiple > > > contributors involved on the PR. > > > > > > Also on the same note, I request contributors(regardless of multiple > > > contributors or not) to keep a clean commit history by squashing the > > > commits and not push all your WIP commits to the PR. this will help u= s > > keep > > > our commit history clean and meaningful. > > > > > > Let me know your thoughts/better approach or If I have misunderstood > how > > > this works. > > > > > > Thanks, Naveen > > > > > > --0000000000005fdb7d0570cc61d3--