[sakai-core-team] [Building Sakai] Question about Pull requests

[Matthew Jones]

Yeah, agreed, kind of what this graph here is talking about no point to
have 10 meaningless commits in the logs.

https://twitter.com/jamesfublo/status/402407321265274881/photo/1

But I'm all for leaving in the good commit messages! +1

On Tue, Feb 10, 2015 at 6:10 PM, Noah Botimer <botimer at umich.edu> wrote:

> As with many things, I see this as an "it depends" situation.
>
> There are plenty of cases where an approach turns out wrong or there are
> just plain mistakes and corrections in the branch. In these, I prefer that
> the branch be rebased into a clean diff without the course corrections.
>
> But I'm not a strict adherent to one-commit-per-branch (or
> one-branch-per-issue). If the work is divided across coherent, cohesive
> commits with good messages, clarity is only harmed by combining them.
>
> So, a rule of thumb that is simple and generally safe seems the way to go
> to me. There is plenty of room for experts to make special case judgment
> calls. If we feel that our average developer should reasonably be familiar
> with interactive rebases, force pushes, and the quirks and risks, therein,
> so be it. If we feel that the general recommendation should be for no
> history changes anywhere, so be it.
>
> Because git affords a new, safer local workflow of committing early and
> often (without shared clutter), I see the interactive rebase of a local
> branch before push as worth recommending. If local history changes only is
> as far as we want to include in the rule of thumb, I'm fine with that, but
> also fine with the more complicated model of history changes in fork
> feature branches. These are transactions with our collective history, so
> they should be the shape and size we want them to be.
>
> Anyway, as I opened, I think it depends -- and we should just make sure to
> use the simplest acceptable pattern as the rule of thumb and make room for
> special cases.
>
> Thanks,
> -Noah
>
> On Feb 10, 2015, at 5:12 PM, Matthew Jones <matthew at longsight.com> wrote:
>
> I think it depends, there really isn't a huge crisis if there just a a few
> commits in a PR and the commits do have good messages tracked back to the
> jira that indicates what specifically has changed between each commit.
>
> I think the problem comes if someone was working on a larger feature and
> if they were doing a commit very frequently and not putting much into the
> message. (Or tracking it as a separate new issue) An example like in the
> Stack Exchange answer that makes sense to me.
>
> http://programmers.stackexchange.com/a/263172
>
> Like if I was going to single out a recent SAK it would be this one
> https://jira.sakaiproject.org/browse/SAK-25773
>
> There are like 50 commits on that and I'd probably be happier if that was
> all just squashed into one or two in the git logs. Taken individually there
> weren't any large changes, but a ton of small ones.
>
>
> On Tue, Feb 10, 2015 at 4:24 AM, Steve Swinsburg <
> steve.swinsburg at gmail.com> wrote:
>
>> I'm not a fan of squashing commits. We didn't do it in subversion and it
>> can break things majorly if you choose the wrong commit, which is easy to
>> do. Many gui clients don't fully support it either. IMO it is unnecessary.
>>
>> What you did was fine, the commit in your branch updated the pr
>> automatically,  then it was merged in the pr. A pr is just a link between
>> the commits in a branch and the base.
>>
>> Cheers,
>> Steve
>>
>> sent from my mobile device
>> On 10/02/2015 7:19 AM, "Noah Botimer" <botimer at umich.edu> wrote:
>>
>>> It's worth mentioning: any time you are planning to use --force,
>>> double-check your work. You could probably recover from a mistake, but it
>>> can be a lot of work.
>>>
>>> Since we are recommending that feature branches are rebased to include
>>> updates, it is good to get in the habit of being explicit. That is, include
>>> the remote name and branch name as a safety check, and try once before
>>> forcing. The various versions of the git clients have different defaults,
>>> you may not have your complete config on whatever machine you're currently
>>> using, or remote tracking might not be what you assumed for a particular
>>> branch.
>>>
>>> Use this:
>>>
>>> git push remote branch-name
>>>  ### Look at the big, nasty error message about divergent branches
>>>  ### Make sure this force is really what you want
>>> git push origin branch-name --force
>>>
>>> Instead of:
>>>
>>> git push --force
>>>
>>> The latter will cost you karma points and major fixup time eventually,
>>> dwarfing the few seconds of typing you saved with the short form.
>>>
>>> Thanks,
>>> -Noah
>>>
>>> On Feb 9, 2015, at 2:55 PM, Brian Jones wrote:
>>>
>>> > Hey Neal,
>>> >
>>> >> I made a pull request off a branch from my clone of Sakai project and
>>> >> then had a suggestion to change my pull request. I made the update in
>>> >> my branch and Git (or Github) seems to have automatically squashed my
>>> >> commits together in the one pull request, which is a good thing.
>>> >
>>> > Not quite. It's automatically updated the PR, but you've made several
>>> > commits (3 in total), that should be squashed into 1. You can refer to
>>> > step 6c in
>>> https://confluence.sakaiproject.org/display/SAKDEV/Git+Setup
>>> > to walk you through squashing commits.
>>> >
>>> >> I don't need to merge my change back into my clone of Sakai project,
>>> >> just after the PR is merged, I can switch my local branch back to
>>> >> origin and git pull upstream master, and then I'm all synche'd up and
>>> >> all is good?
>>> >
>>> > Correct, after the PR is merged, you can just switch back to your
>>> master
>>> > branch and do the git pull upstream master && git push origin master to
>>> > both update your fork on your local machine and your fork in github.
>>> >
>>> > Hope this helps :)
>>> >
>>> > Brian Jones
>>> > Programmer/Analyst
>>> > Information Technology Services
>>> > Support Services Building, Suite 4300
>>> > Western University
>>> > (519) 661-2111 x86969
>>> > bjones86 at uwo.ca
>>> > _______________________________________________
>>> > sakai-core-team mailing list
>>> > sakai-core-team at collab.sakaiproject.org
>>> > http://collab.sakaiproject.org/mailman/listinfo/sakai-core-team
>>>
>>> _______________________________________________
>>> sakai-core-team mailing list
>>> sakai-core-team at collab.sakaiproject.org
>>> http://collab.sakaiproject.org/mailman/listinfo/sakai-core-team
>>>
>>
>> _______________________________________________
>> sakai-dev mailing list
>> sakai-dev at collab.sakaiproject.org
>> http://collab.sakaiproject.org/mailman/listinfo/sakai-dev
>>
>> TO UNSUBSCRIBE: send email to
>> sakai-dev-unsubscribe at collab.sakaiproject.org with a subject of
>> "unsubscribe"
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://collab.sakaiproject.org/pipermail/sakai-core-team/attachments/20150210/07df74ef/attachment.html