Smalltalk › Squeak › Squeak VM

Re: [OpenSmalltalk/opensmalltalk-vm] FileAttributesPlugin: Add session Id (f6445ab)

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

5 messages Options

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] FileAttributesPlugin: Add session Id (f6445ab)

Hi Ben,

On Mon, 17 Dec 2018 at 15:55, Ben Coman <[hidden email]> wrote:
>
> To provide cleaner PRs, I tend to "git commit --amend" to my-branch that I
> issued a PR from and force push that to my github account.
> In spite of the general advice not to force push public branches, in
> reality it seems people rarely branch except from the canonical account.
> Actually this could be covered by a convention like...
> "if you branch from someone's personal account, immediately push it to
> your account so it shows up in the network view."
> Forcing pushing only a problem if someone has branched from it the network
> view could be checked before forcing pushing.

Just as an alternative:

Instead of amending the commits, sometimes I double branch and squash
the merges back, e.g. (from memory):

$ git checkout Cog
$ git checkout -b IssueBranch
$ git checkout -b IssueBranchDev

Lots of little commits, backtracks, etc. while solving the problem.

$ git checkout IssueBranch
$ git merge --squash IssueBranchDev
$ git push

and then create the PR.

The advantage is that I still have the messy history available in
IssueBranchDev while I'm doing the development.

Cheers,
Alistair

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Nicolas Cellier

Re: [OpenSmalltalk/opensmalltalk-vm] FileAttributesPlugin: Add session Id (f6445ab)

Note that I like to see the "messy" decomposed history where each change has a rationale attached in a separate commit message.

Of course, erratic trial and errors may benefit from a little rewrite of history, but not a squash, it's too violent for my taste, more something like a rebase -i.

When perigrinations are too much random, I prefer a fulll rewrite on a brand new branch with cherry-picking.

Le jeu. 27 déc. 2018 à 16:09, akgrant <[hidden email]> a écrit :

Hi Ben,

On Mon, 17 Dec 2018 at 15:55, Ben Coman <[hidden email]> wrote:
>
> To provide cleaner PRs, I tend to "git commit --amend" to my-branch that I
> issued a PR from and force push that to my github account.
> In spite of the general advice not to force push public branches, in
> reality it seems people rarely branch except from the canonical account.
> Actually this could be covered by a convention like...
> "if you branch from someone's personal account, immediately push it to
> your account so it shows up in the network view."
> Forcing pushing only a problem if someone has branched from it the network
> view could be checked before forcing pushing.

Just as an alternative:

Instead of amending the commits, sometimes I double branch and squash
the merges back, e.g. (from memory):

$ git checkout Cog
$ git checkout -b IssueBranch
$ git checkout -b IssueBranchDev

Lots of little commits, backtracks, etc. while solving the problem.

$ git checkout IssueBranch
$ git merge --squash IssueBranchDev
$ git push

and then create the PR.

The advantage is that I still have the messy history available in
IssueBranchDev while I'm doing the development.

Cheers,
Alistair

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] FileAttributesPlugin: Add session Id (f6445ab)

In reply to this post by David T Lewis

Note that I like to see the "messy" decomposed history where each change
has a rationale attached in a separate commit message.
Of course, erratic trial and errors may benefit from a little rewrite of
history, but not a squash, it's too violent for my taste, more something
like a rebase -i.

I think maybe we're saying the same thing. Each commit in the main branch (Cog) should be single purpose and well documented, e.g. includes the rationale, which allows the reader to see the progression and consume the changes in small, understandable steps. I'm just suggesting one way to achieve that.

Leaving a messy (i.e. something that includes a mistake) decomposed history can just lead to confusion, e.g. I recently didn't squash down one of my PR's. Because it had every intermediate commit one reviewer thought I had committed a plugin built from dirty code, when in fact a later commit in the PR used clean code.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] FileAttributesPlugin: Add session Id (f6445ab)

In reply to this post by David T Lewis

Nicolas, Alistair,

this is a useful conversation. Consider adding it to CONTRIBUTING.md

_,,,^..^,,,_ (phone)

> On Dec 27, 2018, at 10:13 AM, akgrant43 <[hidden email]> wrote:
>
> Note that I like to see the "messy" decomposed history where each change
> has a rationale attached in a separate commit message.
> Of course, erratic trial and errors may benefit from a little rewrite of
> history, but not a squash, it's too violent for my taste, more something
> like a rebase -i.
>
> I think maybe we're saying the same thing. Each commit in the main branch (Cog) should be single purpose and well documented, e.g. includes the rationale, which allows the reader to see the progression and consume the changes in small, understandable steps. I'm just suggesting one way to achieve that.
>
> Leaving a messy (i.e. something that includes a mistake) decomposed history can just lead to confusion, e.g. I recently didn't squash down one of my PR's. Because it had every intermediate commit one reviewer thought I had committed a plugin built from dirty code, when in fact a later commit in the PR used clean code.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

Ben Coman

Re: [OpenSmalltalk/opensmalltalk-vm] FileAttributesPlugin: Add session Id (f6445ab)

In reply to this post by David T Lewis

On Fri, 28 Dec 2018 at 02:13, akgrant43 <[hidden email]> wrote:

Note that I like to see the "messy" decomposed history where each change
has a rationale attached in a separate commit message.
Of course, erratic trial and errors may benefit from a little rewrite of
history, but not a squash, it's too violent for my taste, more something
like a rebase -i.

I think maybe we're saying the same thing. Each commit in the main branch (Cog) should be single purpose and well documented, e.g. includes the rationale, which allows the reader to see the progression and consume the changes in small, understandable steps. I'm just suggesting one way to achieve that.

Leaving a messy (i.e. something that includes a mistake) decomposed history can just lead to confusion, e.g. I recently didn't squash down one of my PR's. Because it had every intermediate commit one reviewer thought I had committed a plugin built from dirty code, when in fact a later commit in the PR used clean code.

I'm in full agreement. Never a full squash, but trying to minimize the noise of "whoops" commits,

or like for the Freetype-2.9.1 windows build I'm working on, experimenting to understand differences between local and CI environment.

I support using `rebase -i`, but it can take a little practice, and there seems some fear floating around of losing code.

`git commit --amend` is a milder form that is only able to affect the previous commit.

cheers -ben