Should I push or request a pull?

 

 
On Sun, Jun 19, 2016 at 8:01 AM, Eliot Miranda <[hidden email]> wrote:
>
> Hi All,
>
>     fixing callbacks on x64 and so too lazy/focussed elsewhere to check whether I should be pushing after each commit.  What is the right process for uploading changes to OpenSmalltalk/vm?
> _,,,^..^,,,_
> best, Eliot
>

In any case, you need to push the changes from your  local repo  up to
the  github repo.

If you cloned from the official-repo (as you probably are), and...

A. you are working in the Cog branch, then all you need to do is push.

B. you are working in a MyFeature branch, then after pushing to
github, you would also make a github pull request from MyFeature
branch to the Cog branch.

Alternatively, you can merge MyFeature branch into your local Cog
branch (rather than rely on github doing in a pull request) and
continue with A.


If (like me) the local repo was cloned from a personal-fork of the
project (on github), and...

C. I am are working in the Cog branch, then I do B.

D. I am working in a MyFeature branch, I do B.

After any push to github, I find it useful to verify what what github
thinks has happened by reviewing
https://github.com/OpenSmalltalk/vm/network.
There may be a few minute delay as the graph is queued to be rebuilt.

cheers -ben

 
On Sat, 2016-06-18 at 17:01 -0700, Eliot Miranda wrote:
>  
> Hi All,
>
>     fixing callbacks on x64 and so too lazy/focussed elsewhere to
> check whether I should be pushing after each commit.  What is the
> right process for uploading changes to OpenSmalltalk/vm?
> _,,,^..^,,,_
> best, Eliot

Push when you want the world to see your changes.  You don't need to be
1:1 on commits to pushes (i.e. you can batch up multiple commits and
then push them all at once if that's your preference, the public repo
will still record each separate commit)  It's also handy if you're
going to be disconnected for a while so you can commit locally as often
as needed and then push them when you get back online.

 

> On 20 Jun 2016, at 09:53, Norbert Hartl <[hidden email]> wrote:
>
>
>
> A commit is associated with a comment. So commits should be rather small and describing the change. Most of the time there are some changes/commits that only make sense when being together. This would be the view from the build server or from the process of having buildable artifacts all the time. So you can see it as freedom to have a commit that breaks the software which can be build after 5 more commits. So no problem for the world if you push 5 commits at once.

This approach breaks tools like git bisect. If a user has something easy/quick to reproduce and a "good" version and "bad" one, he/she can identify the first broken commit almost automatically. If commits are small then it is quite easy for the maintainer to identify the issue and resolve it.

Projects like Linux have the policy that each single commit needs to compile and for my GPRS/EDGE PCU[1] we have the same policy and I can point to a recent example of git bisect used by a user[2]. As a maintainer this reduces the work I have to do.

In the context of spur there is a significant performance regression of our TCAP benchmark from one spur version to another and with git bisect I could do:

git bisect start
git bisect good <SHA1 of better VM>
git bisect bad <SHA1 of first bad VM>

and then commits from good to bad will be picked and all I need to do is:

make
bench

and then decide to mark this commit as:

git bisect bad
git bisect good

and eventually end with the first bad commit (or none if I track noise)


It is obviously up to the maintainers if they want to support git bisect but thanks to git it is not really a burden. One can experiment with temporary branches and then rebase to a final and working patchset. With later git versions can be easily automated as well.

git rebase -i --execute "./compile_and_test_all.sh" origin/master
close the editor..and git starts compiling all your intermediate commits and stops if they fail



kind regards
        holger




[1] A GSM component that converts a LLC frame to smaller pieces. Decides on the coding scheme (error correction bits vs. data), schedules resources across several handsets.
[2] http://lists.osmocom.org/pipermail/openbsc/2016-May/009139.html

 
I've pushed a CONTRIBUTING.md to the repository that we can iterate
over and discuss.

If you go to https://github.com/OpenSmalltalk/vm/commit/5052e307fda0369f04550966fcfdf0a372aeaf39
you can leave comments inline or at the end.

On 19 June 2016 at 02:01, Eliot Miranda <[hidden email]> wrote:
>
> Hi All,
>
>     fixing callbacks on x64 and so too lazy/focussed elsewhere to check whether I should be pushing after each commit.  What is the right process for uploading changes to OpenSmalltalk/vm?
> _,,,^..^,,,_
> best, Eliot
>

 
Tim,

Thank you for posting CONTRIBUTING.md, that seems to provide a very good overview.

Dave

On Mon, Jun 20, 2016 at 11:55:13AM +0200, Tim Felgentreff wrote:

On Mon, Jun 20, 2016 at 5:55 PM, Tim Felgentreff
<[hidden email]> wrote:
>
>
> I've pushed a CONTRIBUTING.md to the repository that we can iterate
> over and discuss.
>
> If you go to https://github.com/OpenSmalltalk/vm/commit/5052e307fda0369f04550966fcfdf0a372aeaf39
> you can leave comments inline or at the end.
>

Thanks Tim, thats a really good starting point.  Usually github would
be the place to comment in-line, but this influences community
workflow a a broader audience is useful.

The thing I want to understand most is the strong discouragement of
rebasing rather than using it to maintain a tidy history.

First there is...
> This also means that you should avoid rebasing or squashing
> your work. Just keep the history.

I happened to look at Laura's PR that adds some good info to
CONTRIBUTING, so I use that as an example...
   https://github.com/OpenSmalltalk/vm/pull/14
Do you really want all four commits to appear in the master history?
I hunted up a dozen articles around this point, and even though a
little long winded, the best was this one...

   http://blog.izs.me/post/37650663670/git-rebase

I particularly liked how the two camps of rebase or not were
characterised into platform or service, as well as the explanation of
bisect.

Then there is...
> Every so many commits it is a good idea to push your work.
> Please refrain from rebasing, unless your history looks like this:

So the need for history cleanup is alluded to, but no guidance is
given on *how* to do it safely.
The critical point to make is to distinguish between cleaning private
branches and leaving public branches alone.  This is a good article
providing some examples...

    https://sandofsky.com/blog/git-workflow.html

Thirdly there is...
> However, if you're unsure what rebasing is, just forget about it
> and push the history as-is.

Again this seems to instil fear of a very useful tool for cleaning up
work in progress commits before submitting a pull request.  But first
the community needs to discuss the degree to which a clean linear
history is beneficial or desired.  Then we might define some recipes
to achieve that.

Finally it says...
> You will be thankful later when you need to use `git bisect` to find where a bug comes from.

But IIUC, bisect works better with a clean and linear history.  Also
from the the repository network graph...

   https://github.com/OpenSmalltalk/vm/network

before the 25 May the SVN history is *very* linear (or is this just an
artefact of the migration?) And after the 19 June (considering only
within the OpenSmalltalk swim lane, i.e. ignoring the personal repo
forks) the history is less linear.  Now branching is one of git's
greatest strengths to encourage participation, but that doesn't mean
every branch needs a permanent record in the trunk history.

So considering one particular workflow case, an afternoons work on
documentation or a quick bug fix, which yet may have several commits
but is only pushed once upon completion -- I believe a reasonable
workflow here would be for  `rebase -i`  to squash down to a single
commit prior to issuing the pull request.

Any subsequent commits to the PR from feedback would remain separate
until it is ready to integrate, when the integrator could squash them
as described here...

   "Merge pull request” [Big Green Button] Considered Harmful
   https://blog.spreedly.com/2014/06/24/merge-pull-request-considered-harmful

However it sounds like you've some personal war stories that might be
useful for us to hear.  To me rebase seems straightforward for certain
defined situations, but maybe there is more about the risks I need to
learn...
> Seriously, skipping bad commits  a bisect is easier than fixing
> someones git tree once they have lost commits to the depths of reflog.
> Especially if their recovery attempts have already triggered a Git GC.

cheers -ben

Hi Holger,


I think this is a very sensible policy and that we should support bisect.  Do you have energy to work with Tim F to update CONTRIBUTING.md to include this policy?  Are there ways of enforcing or advising the policy when using git tools or is it simply something we have to tell users? This latter approach is terribly fragile.

 

 

Hi Ben,

One of your arguments is about history cleaning.
Mostly my discouraging of rebase -i comes from experience with people who made a mistake (e.g. as simple as deleting a line and then getting "lucky" and having git not complain about conflicts during the rebase) to lose a commit entirely. And sometimes they continued working for a while, eventually a git gc was triggered, and then that commit was cleaned up and was lost for good by the time they noticed their work was gone. This happened to people around me more than just once, and then I wasn't able to help because git gc had run. Eventually I gave up and encouraged people who do not want to spend the time to really put in the work and understand how git works and what the internal structures are to not use rebase.

Your other argument is about bisecting. Another experience I had was with people rebasing to "clean up" history by grouping related changes together, but then not checking that each commit still compiles. So the bisect scripts we used in these projects still had to include the logic to skip non-compiling commits. As would any script used on a repository where no one used rebase to remove such 'bad' commits. Secondly, if people tried a bunch of different approaches and finally refactored their code and then squashed it all together, any bisects will only find one large commit as the culprit without explanatory history around it, making debugging harder.

All that being said, if you all think it's safe and we all agree that no one has the right to complain about git if they loose their commits, I won't strongly oppose rebasing.

And finally, regarding the linear history up to now: that's an artifact of how SVN works (it doesn't actually track merges) and thus is reflected in the imported history.

cheers,
Tim

Am 24.06.2016 16:11 schrieb "Ben Coman" <[hidden email]>:
>
>
> On Mon, Jun 20, 2016 at 5:55 PM, Tim Felgentreff
> <[hidden email]> wrote:
> >
> >
> > I've pushed a CONTRIBUTING.md to the repository that we can iterate
> > over and discuss.
> >
> > If you go to https://github.com/OpenSmalltalk/vm/commit/5052e307fda0369f04550966fcfdf0a372aeaf39
> > you can leave comments inline or at the end.
> >
>
> Thanks Tim, thats a really good starting point.  Usually github would
> be the place to comment in-line, but this influences community
> workflow a a broader audience is useful.
>
> The thing I want to understand most is the strong discouragement of
> rebasing rather than using it to maintain a tidy history.
>
> First there is...
> > This also means that you should avoid rebasing or squashing
> > your work. Just keep the history.
>
> I happened to look at Laura's PR that adds some good info to
> CONTRIBUTING, so I use that as an example...
>    https://github.com/OpenSmalltalk/vm/pull/14
> Do you really want all four commits to appear in the master history?
> I hunted up a dozen articles around this point, and even though a
> little long winded, the best was this one...
>
>    http://blog.izs.me/post/37650663670/git-rebase
>
> I particularly liked how the two camps of rebase or not were
> characterised into platform or service, as well as the explanation of
> bisect.
>
> Then there is...
> > Every so many commits it is a good idea to push your work.
> > Please refrain from rebasing, unless your history looks like this:
>
> So the need for history cleanup is alluded to, but no guidance is
> given on *how* to do it safely.
> The critical point to make is to distinguish between cleaning private
> branches and leaving public branches alone.  This is a good article
> providing some examples...
>
>     https://sandofsky.com/blog/git-workflow.html
>
> Thirdly there is...
> > However, if you're unsure what rebasing is, just forget about it
> > and push the history as-is.
>
> Again this seems to instil fear of a very useful tool for cleaning up
> work in progress commits before submitting a pull request.  But first
> the community needs to discuss the degree to which a clean linear
> history is beneficial or desired.  Then we might define some recipes
> to achieve that.
>
> Finally it says...
> > You will be thankful later when you need to use `git bisect` to find where a bug comes from.
>
> But IIUC, bisect works better with a clean and linear history.  Also
> from the the repository network graph...
>
>    https://github.com/OpenSmalltalk/vm/network
>
> before the 25 May the SVN history is *very* linear (or is this just an
> artefact of the migration?) And after the 19 June (considering only
> within the OpenSmalltalk swim lane, i.e. ignoring the personal repo
> forks) the history is less linear.  Now branching is one of git's
> greatest strengths to encourage participation, but that doesn't mean
> every branch needs a permanent record in the trunk history.
>
> So considering one particular workflow case, an afternoons work on
> documentation or a quick bug fix, which yet may have several commits
> but is only pushed once upon completion -- I believe a reasonable
> workflow here would be for  `rebase -i`  to squash down to a single
> commit prior to issuing the pull request.
>
> Any subsequent commits to the PR from feedback would remain separate
> until it is ready to integrate, when the integrator could squash them
> as described here...
>
>    "Merge pull request” [Big Green Button] Considered Harmful
>    https://blog.spreedly.com/2014/06/24/merge-pull-request-considered-harmful
>
> However it sounds like you've some personal war stories that might be
> useful for us to hear.  To me rebase seems straightforward for certain
> defined situations, but maybe there is more about the risks I need to
> learn...
> > Seriously, skipping bad commits  a bisect is easier than fixing
> > someones git tree once they have lost commits to the depths of reflog.
> > Especially if their recovery attempts have already triggered a Git GC.
>
> cheers -ben

On Sun, Jun 26, 2016 at 4:51 AM, Tim Felgentreff
<[hidden email]> wrote:
>
>
> Hi Ben,
>
> One of your arguments is about history cleaning.
> Mostly my discouraging of rebase -i comes from experience with people who made a mistake (e.g. as simple as deleting a line and then getting "lucky" and having git not complain about conflicts during the rebase) to lose a commit entirely. And sometimes they continued working for a while, eventually a git gc was triggered, and then that commit was cleaned up and was lost for good by the time they noticed their work was gone. This happened to people around me more than just once, and then I wasn't able to help because git gc had run. Eventually I gave up and encouraged people who do not want to spend the time to really put in the work and understand how git works and what the internal structures are to not use rebase.

Ah, it never cross my mind to delete a line. So I never hit the problem.

At  https://git-scm.com/docs/git-rebase  I found
    rebase.missingCommitsCheck - If set to "warn", print warnings
about removed commits in interactive mode. If set to "error", print
the warnings and stop the rebase. If set to "ignore", no checking is
done. "ignore" by default.

which you can see in action here...
https://github.com/git/git/blob/cf4c2cfe52be5bd973a4838f73a35d3959ce2f43/git-rebase--interactive.sh
    warn "Warning: some commits may have been dropped" \
             "accidentally."
    warn "Dropped commits (newer to older):"

but that didn't work for me and I had to ask on #git  IRC  to discover
this was introduced in 2.6.0.
I'm only 2.1.4 and current release is 2.9.0.

((Also btw I learnt a new trick, at the top of that last link, click <Blame>,
then search for "missingCommitsCheck" and scroll back up
to click on the commit, and at the bottom of the commit
it says  "master v2.9.0…v2.6.0-rc0" ))



>
> Your other argument is about bisecting. Another experience I had was with people rebasing to "clean up" history by grouping related changes together, but then not checking that each commit still compiles. So the bisect scripts we used in these projects still had to include the logic to skip non-compiling commits.

Good point. Although I do like Holger's
    git rebase -i --execute "./compile_and_test_all.sh" origin/master
but this is probably hard to police in an open source environment.

> As would any script used on a repository where no one used rebase to remove such 'bad' commits.
> Secondly, if people tried a bunch of different approaches and finally refactored their code and then squashed it all together, any bisects will only find one large commit as the culprit without explanatory history around it, making debugging harder.

I wouldn't expect every commit to be squashed, just the minuscule
things.  Logical groups of work should remain.

>
> All that being said, if you all think it's safe and we all agree that no one has the right to complain about git if they loose their commits, I won't strongly oppose rebasing.

Its probably too early for anyone new to git to form an experiential
opinion on whether rebase is safe.   Still the main contributors coudl
choose to be conservative.  I think the CONTRIBUTION page could say
something like...
   "git rebase is a dangerous but useful tool for a narrow range of
well defined tasks.  Receipies for these tasks are presented below.
For project consistency, you should adhere strongly to the recipe and
if something unexpected occurs, do... git rebase --abort.    Please
discuss any deviations on the mail list."

and if that sounds reasonable, I'll follow up with some recipes, one
of which might be check "git --version" and upgrade to latest git to
make rebase a bit safer.

cheers -ben

P.S. while on the topic of disasters we can learn from, this "Disaster
2" story [1] might be a useful read for anyone adapting from SVN.
[1] https://randyfay.com/content/avoiding-git-disasters-gory-story

On Tue, Jun 28, 2016 at 11:41 PM, Ben Coman <[hidden email]> wrote:
It took some work to determine out how to upgrade git, so I share that
for reference...

The easy path didn't work...
$ sudo apt-get dist-upgrade
$ lsb_release -a
Debian GNU/Linux 8.4 (jessie)
$ git --version
2.1.4

So I built from source per...
http://superuser.com/questions/644586/how-to-upgrade-to-latest-git-on-debian-7

$ sudo apt-get install devscripts
$ mkdir ~/devel && cd $_
$ dget http://ftp.de.debian.org/debian/pool/main/g/git/git_2.8.1-1.dsc
$ dpkg-source -x git_2.1.4-2.1.dsc
$ cd git
$ dpkg-checkbuilddeps

$ apt-get build-dep git

$ dch --bpo
$ dpkg-buildpackage -uc -us -b
$ cd ..
$ sudo  dpkg -i git_*.deb   git-man_*.deb
$ git --version
2.8.1


cheers -ben

On Wed, Jun 29, 2016 at 1:38 AM, Ben Coman <[hidden email]> wrote:
So of course I had to experiment to understand properly.
The log is a bit long, but maybe a handy reference...
Each experiment delineated by "#===="

TL;DR - check out the last two examples.


#====SETUP MASTER

$ mkdir testrebase && cd testrebase
$ git init
$ echo M >> blah ; git add blah ; git commit -m M


#====NORMAL SQUASH ONLY, NO DELETES

$ git checkout -b testSquashOnly master
$ for N in 1 2 3 4 ;  do (echo $N >> blah ;  git commit -am $N) ;  done
$ cat blah ;  git log --oneline
    M
    1
    2
    3
    4

    b9784f0 4
    97df945 3
    c57a0e6 2
    77366e2 1
    9b9b515 M

$ git rebase -i master
# in editor, change commit 2 to squash, save, exit
# in second editor, change commit message to "1&2", save, exit

$ cat blah ;  git log --oneline
    M
    1
    2
    3
    4

    15cbb61 4
    6416fd2 3
    3b00e2a 1&2
    4bba92f M

No changes lost.  History cleaned okay.


#====DELETE,ERROR,ABORT

$ git checkout -b testDeleteAbort master
$ for N in 1 2 3 4 ;  do (echo $N >> blah ;  git commit -am $N) ;  done
$ cat blah ;  git log --oneline
     # as above

$ git rebase -i master
# in editor, delete commit 2 line, save, exit
    error: could not apply 97df945... 3
    When you have resolved this problem, run "git rebase --continue".
    If you prefer to skip this patch, run "git rebase --skip" instead.
    To check out the original branch and stop rebasing, run "git
rebase --abort".
    Could not apply 97df94529f50a597489a53bf21f25a9c270226a0... 3

# error??!! ==> panic!!! ==> abort
$ cd rebase --abort
$ cat blah ;  git log --oneline
    master
    1
    2
    3
    4

    b9784f0 4
    97df945 3
    c57a0e6 2
    77366e2 1
    9b9b515 master

Abort on all errors is okay.


#====DELETE,ERROR,EDIT,CONTINUE

$ git checkout -b testDeleteMerge master
$ for N in 1 2 3 4 ;  do (echo $N >> blah ;  git commit -am $N) ;  done
$ cat blah ;  git log --oneline
     # as above

$ git rebase -i master
    # in editor, delete commit 2 line, save, exit
    error: could not apply 97df945... 3

# open editor on file blah
# remove line <<<< ==== >>>> inserted by rebase, to end up with...
      master
      1
      2
      3

$ git add blah;  git rebase --continue
    [detached HEAD 1f5ed82] 3
     1 file changed, 2 insertions(+)
     Successfully rebased and updated refs/heads/test.

$ cat blah ;  git log --oneline
    master
    1
    2
    3
    4

    ef57ae0 4
    1f5ed82 3
    77366e2 1
    9b9b515 master

All changes survived, but there is a risk of losing changes
if wrong lines merged in editor.


#====DELETE,ERROR,CONTINUE(WITHOUT EDIT)

$ git checkout -b testDeleteMerge2 master
$ for N in 1 2 3 4 ;  do (echo $N >> blah ;  git commit -am $N) ;  done
$ cat blah ;  git log --oneline
     # as above

$ git rebase -i master
    # in editor, delete commit 2 line, save, exit
    error: could not apply 97df945... 3

# note, no edit of blah
$ git add blah;  git rebase --continue
    # in editor, save, exit
    [detached HEAD bf742d8] 3
     1 file changed, 5 insertions(+)
    error: could not apply 83209d3... 4

$ git add blah;  git rebase --continue
    # in editor, save, exit
    [detached HEAD 42ab0c6] 4
     1 file changed, 4 insertions(+)
    Successfully rebased and updated refs/heads/testDeleteMerge2.

$ cat blah ;  git log --oneline
    M
    1
    <<<<<<< HEAD
    =======
    2
    3
    <<<<<<< HEAD
    >>>>>>> 7176be1... 3
    =======
    4
    >>>>>>> 83209d3... 4
    42ab0c6 4
    bf742d8 3
    a13429f 1
    4bba92f M

Urgh! No lines actually lost, but an awful mess to clean up.


#====DELETE,ERROR,SKIP

$ git checkout -b testDeleteSkip master
$ for N in 1 2 3 4 ;  do (echo $N >> blah ;  git commit -am $N) ;  done
$ cat blah ;  git log --oneline
     # as above

$ git rebase -i master
    # in editor, delete commit 2 line, save, exit
    error: could not apply 97df945... 3

$ git rebase --skip
    error: could not apply b873786... 4

$ git rebase --skip
    Successfully rebased and updated refs/heads/testDeleteSkip.

$ cat blah ;  git log --oneline
    M
    1
    fc67d5e 1
    4bba92f M

OUCH!! CHANGES LOST!!!!  AAARRRGGHHH!!


#====THE FOLLOWING REQUIRES git --version  > 2.6.0


#====DELETE, missingCommitsCheck WARN

$ git checkout -b testMissingCommitsCheckWarn master
$ for N in 1 2 3 4 ;  do (echo $N >> blah ;  git commit -am $N) ;  done
$ cat blah ;  git log --oneline
     # as above

$ git config rebase.missingCommitsCheck warn
$ git rebase -i master
    # in editor, delete commit 2 line, save, exit
Warning: some commits may have been dropped accidentally.
Dropped commits (newer to older):
 - 304610b 2
To avoid this message, use "drop" to explicitly remove a commit.

Use 'git config rebase.missingCommitsCheck' to change the level of warnings.
The possible behaviours are: ignore, warn, error.

error: could not apply a427113... 3

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".
Could not apply a427113b3137a1e585fcd04dcdd64c899fa4f163... 3

SO WITH WARN WE ARE STILL PRESENTED WITH THE CHOICE TO SKIP, ETC...


#====DELETE, missingCommitsCheck ERROR

$ git checkout -b testMissingCommitsCheckError master
$ for N in 1 2 3 4 ;  do (echo $N >> blah ;  git commit -am $N) ;  done
$ cat blah ;  git log --oneline
     # as above

$ git config rebase.missingCommitsCheck error
$ git rebase -i master
    # in editor, delete commit 2 line, save, exit
Warning: some commits may have been dropped accidentally.
Dropped commits (newer to older):
 - 08d9c7b 2
To avoid this message, use "drop" to explicitly remove a commit.

Use 'git config rebase.missingCommitsCheck' to change the level of warnings.
The possible behaviours are: ignore, warn, error.

You can fix this with 'git rebase --edit-todo'.
Or you can abort the rebase with 'git rebase --abort'.

SO WITH ERROR WE ARE PRESENTED WITH SAFER OPTIONS

cheers -ben.