The proposed action manifests as a commit to the Geode git repository, so a PR is an acceptable vehicle for voting in this case.
> On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt <bschucha...@pivotal.io> wrote: > > I see a lot of plus-ones and a "voting deadline" on this DISCUSS thread and a > request to "vote" using a PR. This all seems out of order to me. Our votes > are supposed to be on the email list, aren't they? and I haven't seen a VOTE > request. > > On 12/20/19 9:33 AM, Nabarun Nag wrote: >> +1 >> >> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols <onich...@pivotal.io> wrote: >> >>> Based on the feedback so far, I will amend the proposal to drop item 2). >>> Therefore, the current ability to create merge commits using command-line >>> git will remain available. >>> >>> The proposal as amended is now: >>>> Change GitHub settings to make "Squash and merge" the default >>>> (by removing “Create a merge commit” option). >>>> >>>> Update the PR template to change the text "Is your initial contribution >>>> a single, squashed commit” to “Is your initial contribution squashed for >>>> ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a >>>> ‘fix’ commit)" >>> >>> As Naba suggested, we can try it, and if we don’t like it, it’s simple to >>> revert. >>> >>> I’ve create a PR for the proposed change here: >>> https://github.com/apache/geode/pull/4513 >>> >>> Please use the PR to vote for against this proposal. It will not be >>> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that time) >>> >>>> On Dec 20, 2019, at 8:31 AM, Ju@N <jujora...@gmail.com> wrote: >>>> >>>> +1 >>>> >>>> On Fri 20 Dec 2019 at 16:18, Owen Nichols <onich...@pivotal.io> wrote: >>>> >>>>> Hi Bruce, this proposal will not waste a single second of your time. It >>>>> just prevents people from accidentally pressing a button that we have >>>>> already agreed should never be pressed, but because we never configured >>> our >>>>> GitHub to match our stated policy, is currently the default. >>>>> >>>>> However, it will save a lot of time and frustration for anyone needing >>> to >>>>> bisect failures, revert, or cherry-pick changes, which has merit even if >>>>> you personally never do any of those three things. >>>>> >>>>> Please start a separate thread if you would like to revisit the >>> community >>>>> decision to require passing PR checks. >>>>> >>>>>> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt <bschucha...@pivotal.io> >>>>> wrote: >>>>>> I agree with Jake. I would go further by saying that I see very little >>>>> merit in this proposal. I think we're getting more and more >>> bureaucratic >>>>> in our process and that it stifles productivity. I was recently forced >>> to >>>>> spend three days fixing tests in which I had changed an import statement >>>>> before they would pass stress testing. I'm glad the tests now pass >>>>> reliably but I was very frustrated by the process. >>>>>> On 12/19/19 4:49 PM, Jacob Barrett wrote: >>>>>>> I’m in agreement with Dan. Changes to the infrastructure to flat out >>>>> prevent things that should be self policing is annoying. This PR review >>>>> lock we have had already cost us valuable time waiting for PR pipelines >>> to >>>>> pass that have no relevance to the commit, like CI work: I’d hat to see >>> yet >>>>> another process enforced that Kees us from getting work done when >>> necessary. >>>>>>> -Jake >>>>>>> >>>>>>> >>>>>>>> On Dec 19, 2019, at 4:43 PM, Dan Smith <dsm...@pivotal.io> wrote: >>>>>>>> >>>>>>>> -1 to (1) and (2). >>>>>>>> >>>>>>>> I think merge commits are appropriate in certain circumstances, so I >>>>> don't >>>>>>>> think we should make a blanket restriction. In fact I think our >>> release >>>>>>>> process involves some merges. >>>>>>>> >>>>>>>> I think setting standards on what is reasonable to be an individual >>>>> commit >>>>>>>> will do a lot more to clean up our history than blocking merges. We'd >>>>>>>> rather not see commits like "Spotless Apply" in the history, but if >>>>>>>> reasonably separate and well written commits come in as part of a >>>>> merge, I >>>>>>>> think that's fine. >>>>>>>> >>>>>>>> -Dan >>>>>>>> >>>>>>>>> On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao <jil...@pivotal.io> >>>>> wrote: >>>>>>>>> +1 >>>>>>>>> >>>>>>>>>> On Thu, Dec 19, 2019, 4:05 PM Owen Nichols <onich...@pivotal.io> >>>>> wrote: >>>>>>>>>> I’d like to advance this topic from an informal request/discussion >>>>> to a >>>>>>>>>> discussion of a concrete proposal. >>>>>>>>>> >>>>>>>>>> To recap, it sounds like there is general agreement that commit >>>>> history >>>>>>>>> on >>>>>>>>>> develop should be linear (no merge commits), and that the biggest >>>>>>>>> obstacle >>>>>>>>>> to this is that the PR merge button in GitHub creates a merge >>> commit >>>>> by >>>>>>>>>> default. >>>>>>>>>> >>>>>>>>>> I propose the following changes: >>>>>>>>>> 1) Change GitHub settings to remove the ability to create a merge >>>>> commit. >>>>>>>>>> This will make Squash & Merge the default. >>>>>>>>>> >>>>>>>>>> 2) Change GitHub settings to require linear history on develop. >>> This >>>>>>>>>> prevents merge commits via command-line (not recommended, but wiki >>>>> still >>>>>>>>>> has instructions for merging PRs this way). >>>>>>>>>> >>>>>>>>>> 3) Update the PR template to change the text "Is your initial >>>>>>>>> contribution >>>>>>>>>> a single, squashed commit” to “Is your initial contribution >>> squashed >>>>> for >>>>>>>>>> ease of review (e.g. a single commit, or a ‘refactoring’ commit >>> plus >>>>> a >>>>>>>>>> ‘fix’ commit)" >>>>>>>>>> >>>>>>>>>> For clarity, I am proposing no-change in the following areas: >>>>>>>>>> i) Leave Rebase & Merge as an option for PRs that have been >>>>> structured to >>>>>>>>>> benefit from it (this can make it easier in a bisect to see whether >>>>> the >>>>>>>>>> refactoring or the “fix” broke something). >>>>>>>>>> ii) Leave existing wording in the wiki as-is [stating that >>> squashing >>>>> is >>>>>>>>>> preferred]. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Please comment via this email thread. >>>>>>>>>> -Owen >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> On Dec 16, 2019, at 10:49 AM, Kirk Lund <kl...@apache.org> wrote: >>>>>>>>>>> >>>>>>>>>>> I think it's already resolved Udo ;) >>>>>>>>>>> >>>>>>>>>>> Here's the problem, if I fixup a dunit test by removing all uses >>> of >>>>>>>>>> "this." >>>>>>>>>>> and I rename the dunit test, then git doesn't remember that the >>> file >>>>>>>>> is a >>>>>>>>>>> rename -- it forever afterwards interprets it as a new file that I >>>>>>>>>> created >>>>>>>>>>> if I touch more than 50% of the lines (which "this." can easily >>>>> do). If >>>>>>>>>> we >>>>>>>>>>> squash two commits: the rename and the cleanup of that dunit test >>> -- >>>>>>>>> then >>>>>>>>>>> we effectively lose the history of that file and it shows that I >>>>>>>>> created >>>>>>>>>> a >>>>>>>>>>> new file. >>>>>>>>>>> >>>>>>>>>>> Also for the record, I've been working on Geode since the >>> beginning >>>>>>>>> and I >>>>>>>>>>> was never made aware of this change in our process. I never voted >>> on >>>>>>>>> it. >>>>>>>>>>> I'm not a big fan of changing various details in our process every >>>>>>>>> single >>>>>>>>>>> week. It's very easy to miss these discussions unless someone >>>>> points it >>>>>>>>>> out >>>>>>>>>>> to me. >>>>>>>>>>> >>>>>>>>>>> On Mon, Dec 16, 2019 at 10:34 AM Udo Kohlmeyer < >>>>> ukohlme...@pivotal.io> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> I'm not sure what this discussion is about... WE, as a community, >>>>> have >>>>>>>>>>>> agreed in common practices, in two place no less... >>>>>>>>>>>> >>>>>>>>>>>> 1) Quoting our PR template >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> For all changes: >>>>>>>>>>>> >>>>>>>>>>>> * >>>>>>>>>>>> >>>>>>>>>>>> Is there a JIRA ticket associated with this PR? Is it referenced >>>>> in >>>>>>>>>>>> the commit message? >>>>>>>>>>>> >>>>>>>>>>>> * >>>>>>>>>>>> >>>>>>>>>>>> Has your PR been rebased against the latest commit within the >>>>>>>>> target >>>>>>>>>>>> branch (typically|develop|)? >>>>>>>>>>>> >>>>>>>>>>>> * >>>>>>>>>>>> >>>>>>>>>>>> ***Is your initial contribution a single, squashed commit?* >>>>>>>>>>>> >>>>>>>>>>>> * >>>>>>>>>>>> >>>>>>>>>>>> Does|gradlew build|run cleanly? >>>>>>>>>>>> >>>>>>>>>>>> * >>>>>>>>>>>> >>>>>>>>>>>> Have you written or updated unit tests to verify your changes? >>>>>>>>>>>> >>>>>>>>>>>> * >>>>>>>>>>>> >>>>>>>>>>>> If adding new dependencies to the code, are these dependencies >>>>>>>>>>>> licensed in a way that is compatible for inclusion underASF 2.0 >>>>>>>>>>>> <http://www.apache.org/legal/resolved.html#category-a>? >>>>>>>>>>>> >>>>>>>>>>>> On our PR template we call out that the initial PR commit should >>> be >>>>>>>>>>>> squashed. >>>>>>>>>>>> >>>>>>>>>>>> 2) >>> https://cwiki.apache.org/confluence/display/GEODE/Code+contributions >>>>>>>>>>>> < >>>>> https://cwiki.apache.org/confluence/display/GEODE/Code+contributions >>>>>>>>>>>> -- See "Accepting a PR Using the Command Line" - Point #3. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> @Kirk, if each of your commits "stands alone", I commend you on >>> the >>>>>>>>>>>> diligence, but in reality, they should either then be stand alone >>>>> PR's >>>>>>>>>>>> or just extra work created for yourself. >>>>>>>>>>>> >>>>>>>>>>>> If we want to change the way we have agreed upon we >>>>>>>>> submit/commit/merge >>>>>>>>>>>> changes back into develop... Then this is another discussion >>>>> thread, >>>>>>>>>>>> until then, I think we should all remind ourselves on our agreed >>>>>>>>>>>> contributions code of conduct. >>>>>>>>>>>> >>>>>>>>>>>> --Udo >>>>>>>>>>>> >>>>>>>>>>>> On 12/16/19 9:59 AM, Nabarun Nag wrote: >>>>>>>>>>>>> Kirk, I believe that creating a Pull Request with multiple >>>>> commits is >>>>>>>>>> ok. >>>>>>>>>>>>> It's just in the end that when it's being pushed to develop >>>>> branch, >>>>>>>>> it >>>>>>>>>>>>> needs to be squash merged. I believe that is what you have >>>>> mentioned >>>>>>>>> in >>>>>>>>>>>> the >>>>>>>>>>>>> first paragraph, and I am more than happy with that. >>>>>>>>>>>>> If you can see in the first screenshot comparison that I had >>>>> attached >>>>>>>>>> in >>>>>>>>>>>>> the first email in this thread is what I want to avoid. >>>>>>>>>>>>> >>>>>>>>>>>>> Thank you for your feedback. >>>>>>>>>>>>> >>>>>>>>>>>>> Regards >>>>>>>>>>>>> Naba >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On Mon, Dec 16, 2019 at 9:47 AM Kirk Lund <kl...@apache.org> >>>>> wrote: >>>>>>>>>>>>>> Whenever I submit a PR with multiple commits that I intend to >>>>> rebase >>>>>>>>>> to >>>>>>>>>>>>>> develop, I always try to ensure that each commit stands alone >>> as >>>>> is >>>>>>>>>>>>>> (compiles and passes tests). Separating file renames and >>>>> refactoring >>>>>>>>>>>> from >>>>>>>>>>>>>> behavior changes into different commits seems very valuable to >>>>> me, >>>>>>>>> and >>>>>>>>>>>> I've >>>>>>>>>>>>>> had trouble getting people to review PRs without this >>> separation >>>>>>>>> (but >>>>>>>>>> it >>>>>>>>>>>>>> could be squashed as it's merged to develop). >>>>>>>>>>>>>> >>>>>>>>>>>>>> It sounds to me like the real problem is (a) some PRs have >>>>> multiple >>>>>>>>>>>> commits >>>>>>>>>>>>>> that don't compile or don't pass tests, and (b) some PRs that >>>>> should >>>>>>>>>> be >>>>>>>>>>>>>> merged with squash are not (by accident most likely). >>>>>>>>>>>>>> >>>>>>>>>>>>>> I can submit multiple PRs instead of one PR with multiple >>>>> commits. >>>>>>>>> So >>>>>>>>>>>> I'll >>>>>>>>>>>>>> change my response to -0 if that helps prevent commits to >>> develop >>>>>>>>> that >>>>>>>>>>>>>> don't compile or pass tests. Without preventing rebase or merge >>>>>>>>>> commits >>>>>>>>>>>>>> from github, I'm not sure how we can really enforce this or >>>>> prevent >>>>>>>>>> (b) >>>>>>>>>>>>>> above. >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Fri, Dec 13, 2019 at 3:38 PM Alexander Murmann < >>>>>>>>>> amurm...@pivotal.io> >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> I wonder if Kirk's and Naba's statements are necessarily at >>>>> odds. >>>>>>>>>>>>>>> Make the change easy (warning: this may be hard), then make >>> the >>>>>>>>> easy >>>>>>>>>>>>>>>> change." >>>>>>>>>>>>>>> -Kent Beck >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Following Kent Beck's advise might reasonably split into two >>>>>>>>> commits. >>>>>>>>>>>> One >>>>>>>>>>>>>>> refactor commit and a separate commit that introduces the >>> actual >>>>>>>>>>>> change. >>>>>>>>>>>>>>> They should be individually revertible and might be easier >>>>>>>>> understood >>>>>>>>>>>> if >>>>>>>>>>>>>>> split out. I vividly remember a change on our code base where >>>>>>>>> someone >>>>>>>>>>>>>> did a >>>>>>>>>>>>>>> huge amount of refactoring that resulted than in one parameter >>>>>>>>>> changing >>>>>>>>>>>>>> in >>>>>>>>>>>>>>> order to get the desired functionality change. If that was in >>>>> one >>>>>>>>>>>> commit, >>>>>>>>>>>>>>> it would be hard to see the actual change. If split out, it's >>>>>>>>>> beautiful >>>>>>>>>>>>>> and >>>>>>>>>>>>>>> crystal clear. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> I am unsure how that would be reflected in terms of JIRA >>> ticket >>>>>>>>>>>>>> references. >>>>>>>>>>>>>>> Usually we assume that if there is a commit with the ticket >>>>> number, >>>>>>>>>> the >>>>>>>>>>>>>>> issue is resolved. Maybe the key here is to create a separate >>>>>>>>>>>> refactoring >>>>>>>>>>>>>>> ticket. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Would that allow us to have our cake and eat it too? >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Fri, Dec 13, 2019 at 3:16 PM Nabarun Nag <n...@pivotal.io> >>>>>>>>> wrote: >>>>>>>>>>>>>>>> It is to help with bisect operations when things start >>> failing >>>>> ... >>>>>>>>>>>>>> helps >>>>>>>>>>>>>>> us >>>>>>>>>>>>>>>> it revert and build faster. >>>>>>>>>>>>>>>> also with cherry picking features / fixes to previous >>> versions >>>>> . >>>>>>>>>>>>>>>> And keeping the git history clean with no unnecessary “merge >>>>>>>>>> commits”. >>>>>>>>>>>>>>>> Regards >>>>>>>>>>>>>>>> Naba >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Fri, Dec 13, 2019 at 2:25 PM Kirk Lund <kl...@apache.org> >>>>>>>>> wrote: >>>>>>>>>>>>>>>>> -1 I really like to sometimes have more than 1 commit in a >>> PR >>>>> and >>>>>>>>>>>>>> keep >>>>>>>>>>>>>>>> them >>>>>>>>>>>>>>>>> separate when they merge to develop >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On Tue, Oct 22, 2019 at 5:12 PM Nabarun Nag < >>> n...@pivotal.io> >>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>> Hi Geode Committers, >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> A kind request for using squash commit instead of using >>>>> merge. >>>>>>>>>>>>>>>>>> This will really help us in our bisect operations when a >>>>>>>>>>>>>>>>>> regression/flakiness in the product is introduced. We can >>>>>>>>> automate >>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>> go >>>>>>>>>>>>>>>>>> through fewer commits faster, avoiding commits like >>> "spotless >>>>>>>>> fix" >>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>> "re-trigger precheck-in" or other minor commits in the >>> merged >>>>>>>>>>>>>> branch. >>>>>>>>>>>>>>>>>> Also, please use the commit format : (helps us to know who >>>>>>>>> worked >>>>>>>>>>>>>> on >>>>>>>>>>>>>>>> it, >>>>>>>>>>>>>>>>>> what is the history) >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> * GEODE-xxxx: <brief intro > >>>>>>>>>>>>>>>>>> * explanation line 1 * >>>>>>>>> explanation >>>>>>>>>>>>>>> line >>>>>>>>>>>>>>>> 2* >>>>>>>>>>>>>>>>>> This is not a rule or anything, but a request to help out >>>>> your >>>>>>>>>>>>>> fellow >>>>>>>>>>>>>>>>>> committers in quickly detecting a problem. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> For inspiration, we can look into Apache Kafka / Spark >>> where >>>>>>>>> they >>>>>>>>>>>>>>> have >>>>>>>>>>>>>>>> a >>>>>>>>>>>>>>>>>> complete linear graph for their main branch HEAD [see >>>>>>>>> attachment] >>>>>>>>>>>>>>>>>> Regards >>>>>>>>>>>>>>>>>> Naba. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>> -- >>>> Ju@N >>>
