+1 for #1 with exception (option 3) for features development. For
majority of bug fixes single commit is much easier to handle especially
that pull request is not reflected in version control/git and multiple
commits for a logical change will lead to more confusion as they may be
committed at different time and be intermixed with other commits. There
are exceptions where it will make sense to break one large commit into
few smaller ones (option #3) that more likely will apply to large
features development with few contributors to it. In this case each
commit should still represent logically complete change/addition.
Some portion of pull requests reviews may need to be reflected
(duplicated?) in JIRA or code comments, depending on the impact on
feature/patch. This should partially eliminate pros of #2. This brings
additional note on #3. It should be less combination of #1 and #2
(assuming that #2 is purely documenting review history) and more
breaking large commit into smaller logically complete work.
Thank you,
Vlad
On 11/2/15 11:53, Ganelin, Ilya wrote:
I think while the patch is in progress, commits should be visible, and work
should be spread across commits such that individual changes are easier to
track.
I think when merging to master, it’s ok to squash the final result to a commit
message, assuming that the PR itself contains a detailed summary (in the
introduction) of what was done and what changes were made. This, coupled with
the discussion of approach and background in the JIRA should ensure we have
good coverage of intent, history, and changes.
On Nov 2, 2015, at 11:30 AM, Timothy Farkas <[email protected]> wrote:
Yes Pramod I must understood your suggestion. I prefer option 1, with the
exception of cases where multiple commits are required to preserve
attribution.
Utlimately I think the choice depends on what function you want a branch to
serve. If you want the branch to tell the story of how the code was
developed then option 2 or 3 is superior. If you want the branch to
represent purely functional changes that can be easily ported, then option
1 is superior. I prefer the latter view of a branch.
Tim
On Mon, Nov 2, 2015 at 11:11 AM, Pramod Immaneni <[email protected]>
wrote:
Tim are you saying at the end of the process you should only see one commit
for a JIRA/change. If that is the case then you may be referring to 1.
On Mon, Nov 2, 2015 at 11:09 AM, Timothy Farkas <[email protected]>
wrote:
+1 for 3. After review I think everything should still be squashed into 1
commit, with some exceptions like preserving attribution (for example
when
you rename a file or when multiple authors work on a feature).
On Mon, Nov 2, 2015 at 11:00 AM, Pramod Immaneni <[email protected]
wrote:
I wanted to find out how folks feel about squashing commits for reviews
on
pull request. This is not for the initial pull request but only for
subsequent commits to address the reviews. Here are some options.
1. Squash everything to a single commit. This is the process we are
following today. Advantage is there is one commit per JIRA and self
contained. Easy to cherry-pick if needed.
2. Preserve the individual commits for pull request reviews. Advantage
is
you preserve the review history in the pull request, the thoughts and
discussions that went behind the changes and you see the incremental
changes in separate commits. Disadvantage is you will have to work with
multiple commits if you are trying to something with the change like
re-apply it elsewhere.
3. A hybrid of 1. and 2. where you don't let the commits grow large. No
standard set limit but the contributor and committer work keep it to a
reasonable amount squashing smaller commits.
4. Anything you would like to propose.
I would like to add +1 for option 3.
Thanks
________________________________________________________
The information contained in this e-mail is confidential and/or proprietary to
Capital One and/or its affiliates and may only be used solely in performance of
work or services for Capital One. The information transmitted herewith is
intended only for use by the individual or entity to which it is addressed. If
the reader of this message is not the intended recipient, you are hereby
notified that any review, retransmission, dissemination, distribution, copying
or other use of, or taking of any action in reliance upon this information is
strictly prohibited. If you have received this communication in error, please
contact the sender and delete the material from your computer.
