Page 3 of 4

Re: Pull Request HowTo

Posted: Tue May 02, 2017 10:51 am
by looo
keithsloan52 wrote:git rebase master offset2
this will rebase offset2-branch on your master branch. To rebase on FreeCAD/master you can do the following:

1. add FreeCAD as a remote:

Code: Select all

git remote add FreeCAD https://github.com/FreeCAD/FreeCAD
2. update changes from the master:

Code: Select all

git fetch FreeCAD
3. rebase:

Code: Select all

git rebase FreeCAD/master

Re: Pull Request HowTo

Posted: Tue May 02, 2017 12:20 pm
by keithsloan52
Do I do my changes before or after the rebase?

Re: Pull Request HowTo

Posted: Tue May 02, 2017 5:46 pm
by yorik
When I screw things too much with git I sometimes do the following to start over:

1. Save your changes in a diff file with git diff mybranch..master > mychanges.patch
2. Delete your dirty branch, make a new clean one from a fresh, up-to-date master
3. Switch to that branch and apply your changes with patch -d1 < mychanges.patch
4. If there are changed files produced by diff that shouldn't be changed, git checkout them

Then you have a rebased branch with only your changes there, ready to be committed.

Re: Pull Request HowTo

Posted: Wed May 03, 2017 6:21 am
by Jee-Bee
Thanks i didn't know this was possible!!

Re: Pull Request HowTo

Posted: Wed May 03, 2017 7:24 am
by looo
keithsloan52 wrote:Do I do my changes before or after the rebase?
rebase can be done at any point. But normally you rebase before you do a PR.

Re: Pull Request HowTo

Posted: Fri May 05, 2017 8:15 pm
by CADennis
yorik wrote:When I screw things too much with git I sometimes do the following to start over:

1. Save your changes in a diff file with git diff mybranch..master > mychanges.patch
2. Delete your dirty branch, make a new clean one from a fresh, up-to-date master
3. Switch to that branch and apply your changes with patch -d1 < mychanges.patch
4. If there are changed files produced by diff that shouldn't be changed, git checkout them

Then you have a rebased branch with only your changes there, ready to be committed.
How would you write an automated "merge officer" scan? It must identify any two developers repetitively skipping the 4th step by lack of time/discipline. I need a solution for my ongoing CI enhancements.

Here is an example of this problem: Two musicians, one from Germany, the other from the USA. They write a song that simply traverses the c major scale. They commit note after note. In Germany we write "cdefgahc". Notice the "h"! In the USA musicians write "cdefgabc". Notice the "b"!

The song might internationally evolve as follows:

Code: Select all

USA    :   (cdef)         (bc)---------(de)-----(fga)------(bc)
                 \       /   \           \         \         \
master :         ( )   ( )   ( )   ( )   ( )  ( )  ( )  ( )  ( )
                   \   /           /          /         /
Germany:            (ga)-------(hcd)------(efg)------(ah)
Both get lazy from (ga) onwards. The master would have the effective states

Code: Select all

(cdef)->(cdefga)->(cdefgabc)->(cdefgahcd)->(cdefgabcde)->(cdefgahcdefg)->(cdefgabcdefga)->(cdefgahcdefgah)->(cdefgabcdefgabc)
Other musicians, who care maybe only about the first octave and try to stay synchronized, might get pretty annoyed and confused about these frequent, repetitive overwrites of

Code: Select all

h->b->h->b->h-> ...
I think we must somehow query git over several commits by the same developer to identify that he repeatedly overwrites the master with his unsynchronized way of writing things. I expect that such identification would usually come at least in pairs of developers running into such a loop, and then the "merge officer" must reject them immediately, maybe even rewind a bit. Any snippets or seen a similar automation (static analysis) already?

Re: Pull Request HowTo

Posted: Sat May 06, 2017 11:38 pm
by kkremitzki
Hmm, that seems like a lot more complication than should be necessary. What problem in FreeCAD development necessitates this?

Re: Pull Request HowTo

Posted: Mon May 08, 2017 8:38 pm
by CADennis
c'mon, that's not complicated! That's just two guys writing down the C Major scale :D :D
Maybe your mobile browser didn't render the monospaced font nicely?
monospaced_desktop.png
monospaced_desktop.png (3.62 KiB) Viewed 26589 times
Here is a shell script to demo and have something to implement the scanner against:

Code: Select all

#!/bin/bash
# Copyright 2017, CADennis, FreeCAD Community, 
# Licensed under Creative Commons BY
# ============================================================================
# The song might internationally evolve as follows:
#
# USA    :   (cdef)         (bc)---------(de)-------(fga)--------(bc)
#                  \       /   \           \           \           \
# master :         ( )   ( )   ( )   ( )   ( )   ( )   ( )   ( )   ( )
#                    \   /           /           /           /
# Germany:            (ga)-------(hcd)-------(efg)--------(ah)
# 
# Both get lazy from (ga) onwards. The master would have the effective states
#
# (cdef)->(cdefga)->(cdefgabc)->(cdefgahcd)->(cdefgabcde)->(cdefgahcdefg)->
# -> (cdefgabcdefga)->(cdefgahcdefgah)->(cdefgabcdefgabc)
#
# Other musicians, who care maybe only about the first octave and 
# try to stay synchronized, might get pretty annoyed and confused about 
# these frequent, repetitive overwrites of 
#
# h->b->h->b->h-> ...
# ============================================================================
mkdir torwaltz
cd torwaltz
mkdir protocol
mkdir torwaltz_git
cd torwaltz_git
git init
echo "" > song.txt
git add song.txt
git commit -m "let's make music"
# ====
git checkout -b usa
echo "cdef" >> song.txt
git add song.txt 
git commit -m "cdef - with love from USA"
# ====
git checkout master
git merge usa
NOW=$(date +%Y_%m_%d-%H%M--%S.%3N)
SNAPSHOT=`git rev-parse --short HEAD`
cp song.txt ../protocol/master_snapshot-${NOW}_[${SNAPSHOT}]
# ====
git checkout -b germany
echo "ga" >> song.txt
git add song.txt 
git commit -m "ga - with love from Germany"
# ====
git checkout master
git merge germany
NOW=$(date +%Y_%m_%d-%H%M--%S.%3N)
SNAPSHOT=`git rev-parse --short HEAD`
cp song.txt ../protocol/master_snapshot-${NOW}_[${SNAPSHOT}]
# ====
git checkout usa
echo "bc" >> song.txt
git add song.txt 
git commit -m "bc - with love from USA"
git rebase master 
sed -i '/<<<<<<</d' song.txt
sed -i '/>>>>>>>/d' song.txt
sed -i '/=======/d' song.txt
git add song.txt
git rebase --continue
# ====
git checkout master
git merge usa
NOW=$(date +%Y_%m_%d-%H%M--%S.%3N)
SNAPSHOT=`git rev-parse --short HEAD`
cp song.txt ../protocol/master_snapshot-${NOW}_[${SNAPSHOT}]
# ====
echo "Both get lazy now"
# ====
git checkout germany
echo "hcd" >> song.txt
git add song.txt 
git commit -m "hcd - with love from Germany"
git diff germany..master > mychanges.patch
ls -lsa
git checkout master
git branch -D germany
git checkout -b germany
ls -lsa
patch -R -d . < mychanges.patch
rm mychanges.patch
git add song.txt
git commit -m "hcd - with love from Germany"
# ====
git checkout master
git merge germany
NOW=$(date +%Y_%m_%d-%H%M--%S.%3N)
SNAPSHOT=`git rev-parse --short HEAD`
cp song.txt ../protocol/master_snapshot-${NOW}_[${SNAPSHOT}]
# ====
git checkout usa
echo "de" >> song.txt
git add song.txt 
git commit -m "de - with love from USA"
git diff usa..master > mychanges.patch
ls -lsa
git checkout master
git branch -D usa
git checkout -b usa
ls -lsa
patch -R -d . < mychanges.patch
rm mychanges.patch
git add song.txt
git commit -m "de - with love from USA"
# ====
git checkout master
git merge usa
NOW=$(date +%Y_%m_%d-%H%M--%S.%3N)
SNAPSHOT=`git rev-parse --short HEAD`
cp song.txt ../protocol/master_snapshot-${NOW}_[${SNAPSHOT}]
# ====
git checkout germany
echo "efg" >> song.txt
git add song.txt 
git commit -m "efg - with love from Germany"
git diff germany..master > mychanges.patch
ls -lsa
git checkout master
git branch -D germany
git checkout -b germany
ls -lsa
patch -R -d . < mychanges.patch
rm mychanges.patch
git add song.txt
git commit -m "efg - with love from Germany"
# ====
git checkout master
git merge germany
NOW=$(date +%Y_%m_%d-%H%M--%S.%3N)
SNAPSHOT=`git rev-parse --short HEAD`
cp song.txt ../protocol/master_snapshot-${NOW}_[${SNAPSHOT}]
# ====
git checkout usa
echo "fga" >> song.txt
git add song.txt 
git commit -m "fga - with love from USA"
git diff usa..master > mychanges.patch
ls -lsa
git checkout master
git branch -D usa
git checkout -b usa
ls -lsa
patch -R -d . < mychanges.patch
rm mychanges.patch
git add song.txt
git commit -m "fga - with love from USA"
# ====
git checkout master
git merge usa
NOW=$(date +%Y_%m_%d-%H%M--%S.%3N)
SNAPSHOT=`git rev-parse --short HEAD`
cp song.txt ../protocol/master_snapshot-${NOW}_[${SNAPSHOT}]
# ====
git checkout germany
echo "ah" >> song.txt
git add song.txt 
git commit -m "ah - with love from Germany"
git diff germany..master > mychanges.patch
ls -lsa
git checkout master
git branch -D germany
git checkout -b germany
ls -lsa
patch -R -d . < mychanges.patch
rm mychanges.patch
git add song.txt
git commit -m "ah - with love from Germany"
# ====
git checkout master
git merge germany
NOW=$(date +%Y_%m_%d-%H%M--%S.%3N)
SNAPSHOT=`git rev-parse --short HEAD`
cp song.txt ../protocol/master_snapshot-${NOW}_[${SNAPSHOT}]
# ====
git checkout usa
echo "bc" >> song.txt
git add song.txt 
git commit -m "bc - with love from USA"
git diff usa..master > mychanges.patch
ls -lsa
git checkout master
git branch -D usa
git checkout -b usa
ls -lsa
patch -R -d . < mychanges.patch
rm mychanges.patch
git add song.txt
git commit -m "bc - with love from USA"
# ====
git checkout master
git merge usa
NOW=$(date +%Y_%m_%d-%H%M--%S.%3N)
SNAPSHOT=`git rev-parse --short HEAD`
cp song.txt ../protocol/master_snapshot-${NOW}_[${SNAPSHOT}]
# ====
git log --graph --all --oneline
git status
echo "=========="
echo "let's see HEAD (all 'b' - you wouldn't notice anything)"
cat song.txt
echo "=========="
echo "let's see HEAD~1 (all 'h' - oops, how is that possible?!)"
git checkout -q HEAD~1
cat song.txt
echo "=========="
echo "let's see HEAD~2 (all 'h' - why that, why no 'fga' from USA?!)"
git checkout -q HEAD~2
cat song.txt
echo "=========="
echo "let's see germany (all 'h' - consistent in himself, but not synced)"
git checkout germany
cat song.txt
echo "=========="
echo "If you dare, have a look at the toggling h->b->h->b->... in the protocoled effective master snapshots:"
cd ../protocol
pwd
ls -l
echo "=========="
echo "Any 3rd developer trying to sync with these two bad guys would have given up ;-)"

Re: Pull Request HowTo

Posted: Mon May 08, 2017 9:03 pm
by yorik
Oh we have "merge officiers" already. They are mostly Werner, and a bit myself when my limited knowledge allows it... They work quite well :D

There is a part you cannot totally solve I think... It would be unrealistic to imagine that you will find an automatic system that removes all errors that a coder can commit. If it's not an error like you described, it will be another... It seems to me that the best defenses you can build against errors are a good test suite. This works wonderfully with pull requests, each pull request must pass the whole test suite, that's already a fairly good guarantee that it didn't screw something serious.

Then add a couple of simple rules, like, don't make gigantic pull requests but divide things in smaller parts that are easier to review, etc.

Re: Pull Request HowTo

Posted: Mon May 08, 2017 9:31 pm
by sgrogan
CADennis wrote:Any snippets or seen a similar automation (static analysis) already?
Yes, it's called "git rebase"
Please see here: https://www.atlassian.com/git/tutorials ... s-rebasing and wmayer's most recent previous post in this thread. wmayer and yorik do a great job helping to merge when a contributor has trouble with git, but I think the tools are there, if not easy to use.