Agree with Vlad. #1 is the default and the target we aim to. #3 for special cases.
> On Nov 2, 2015, at 1:00 PM, Vlad Rozov <[email protected]> wrote: > > +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. >> >
