Re: what is the best way to update a geode pull request

+1 for doing whatever facilitates reviewing and is best for the PR (which may vary by PR or even reviewers) -1 to disallowing or even strongly discouraging force pushing in a PR (go ahead and merge or rebase as each person prefers but don't force that preference on others) I prefer to separate

Re: what is the best way to update a geode pull request

+1 to updating the PR template. I've noticed that few people actually follow it and sometimes they just remove it altogether. +1 to pushing PR changes as separate commits. This makes PR review easier. Sometimes it's helpful to me as a reviewer for the initial PR to be split into multiple commits

Re: what is the best way to update a geode pull request

> On May 31, 2019, at 2:40 PM, Udo Kohlmeyer wrote: > > If we are concerned about the single line that can break the product, then > our testing has failed us on so many levels, that there is no hope. Sorry, I used a hyperbolic statement about looking for 1 line out of 1000. The point was

Re: what is the best way to update a geode pull request

> > This sounds to me like "all my friends are jumping off bridges" and an > incorrect application or understanding of your toolset. In my own > experience, I have had much more trouble trying to track a bug through > massive squashed commits than I have identifying which branch was > responsible

Re: what is the best way to update a geode pull request

If we are concerned about the single line that can break the product, then our testing has failed us on so many levels, that there is no hope. But looking forward to see how long one can sustain the "factor -> commit -> make changes required to fulfill JIRA -> commit -> manual merge"...

Re: what is the best way to update a geode pull request

> On May 31, 2019, at 2:23 PM, Udo Kohlmeyer wrote: > I must be honest, but I am yet to find 1 developer that keeps a list of all > changes they want to be refactored separate from the bug/feature code. OR > better stated I am yet to find where this was sustainable AND productive. Challenge

Re: what is the best way to update a geode pull request

I think a single (squashed if required) commit for the start of a PR. ANY changes due to comments should be reflected as separate commits on the PR. Once approved, that PR should squash all commits into 1 commit with a message that describes what was done,etc... In the past I've heard

Re: what is the best way to update a geode pull request

> Not sure if some people just force-push out of habit, or if the requirement for initial commit to be squashed creates some fear. If anyone here finds they have that habit, it would be a good one to break. > I would only do a rebase and force-push if github tells me that there is a > conflict,

Re: what is the best way to update a geode pull request

I really don't like force-push to PRs after comments have been made. At least GitHub shows that the force-push occured now, but that still blocks us from seeing the incremental change. For needing to update a PR due to develop having progressed, I recommend using `git merge origin/develop `

Re: what is the best way to update a geode pull request

+1 to updating the checklist. About single vs multiple commits: I think either is fine as long as it is deliberate and makes it easier to understand what someone is doing. If there is a large refactor that enables a small change, it is often easier to understand when these changes are in separate

Re: what is the best way to update a geode pull request

In my opinion, I am okay will multiple commits within a PR. But please do squash them to a single commit when it is pushed to develop. This helps us a ton if it is single commit. - bisect operations are easier when it is a single commit during major failure analysis. - cherrypick is easier when

Re: what is the best way to update a geode pull request

Would that be PR of a the PR template. Might that cause a black hole to form? I’m in favor of updating this wall of text you smack your face on for each PR. Let’s pair it down to discourage people (myself strongly included) from ignoring or deleting it. -jake > On May 31, 2019, at 1:33 PM,

Re: what is the best way to update a geode pull request

+1 for initial PR needs to be a single commit +1 for making intermittent commits to the PR to show history of revision. I would only do a rebase and force-push if github tells me that there is a conflict, but my rebase would keep my original commits (no squashing). I do not like to do a merge,

Re: what is the best way to update a geode pull request

I’ve almost never seen a PR where this checklist was filled out. Either the PR is created with the original boilerplate intact (or sometimes it’s deleted entirely). I feel like this wall of boilerplate discourages contributors from instead using that space to describe their change and add

Re: what is the best way to update a geode pull request

Let’s update the checklist to match the outcome of this thread: https://github.com/apache/geode/blob/develop/.github/PULL_REQUEST_TEMPLATE.md Anthony > On May 31, 2019, at 1:31 PM, Helena Bales wrote: > > +1. I

Re: what is the best way to update a geode pull request

+1. I would guess that it is the checklist as part of the PR that is confusing people. The other reason that history gets rewritten is when force pushing after a rebase. While fast-forwarding is necessary on occasion, this can be accomplished without rewriting history by using a merge. As part

Re: what is the best way to update a geode pull request

+1 to pushing PR changes as separate commits. Also +1 to creating PRs with multiple commits where it makes sense. -Dan On Fri, May 31, 2019 at 1:28 PM Owen Nichols wrote: > Personally, I do not force-push to my PRs once any review comments have > been accumulated, for the reasons you mention.

Re: what is the best way to update a geode pull request

Personally, I do not force-push to my PRs once any review comments have been accumulated, for the reasons you mention. Not sure if some people just force-push out of habit, or if the requirement for initial commit to be squashed creates some fear. I would go a step further and suggest that