2017-06-30 17:58 GMT+02:00 Ben Coman <btc@openinworld.com>:
On Fri, Jun 30, 2017 at 4:28 PM, Pavel Krivanek <pavel.krivanek@gmail.com> wrote:
Hi,
the reviewing of the pull requests is currently not as easy as we would like to have. For simple changes it should be enough to go to the pull requests list (https://github.com/pharo-project/pharo/pulls) and review the changes from the GitHub web interface. In case that the PR passes tests and bootstrapping, it should be fine.
However if you want to play with an image that contains the PR code, your options are more complicated. The basic and the most clean one is to bootstrap from the repository with the PR applied on it. But it takes A LOT of time (over 20 mins). You can use this script that you will run inside the clone.
set -e
export PR=122 export BRANCH=development
export PHARO_VERSION=60 export BOOTSTRAP_ARCH=32
git fetch origin $BRANCH git fetch origin refs/pull/$PR/head git checkout -b pullrequest FETCH_HEAD git merge --no-edit origin/$BRANCH #git diff origin/$BRANCH..pullrequest > pullrequest.diff
wget -O - get.pharo.org/${PHARO_VERSION}+vm <http://get.pharo.org/$%7BPHARO_VERSION%7D+vm> | bash
./pharo Pharo.image --no-default-preferences ./bootstrap/scripts/ prepare_image.st --save --quit ./pharo Pharo.image --no-default-preferences ./bootstrap/scripts/ bootstrap.st --ARCH=${BOOTSTRAP_ARCH} --quit bash ./bootstrap/scripts/build.sh
The other option is to take existing Pharo 7 image, have pharo-project/pharo clone next to it in a folder named 'pharo-core' and then create a new branch with the merged pull request. Currently it cannot be done from Iceberg directly so you need to use Git. You can use this script:
set -e
PR=119 COMMIT=2225314a7404c9e00fe71e70d41fc59d4ba78e2d
where does this commit hash come from?
as described, SystemVersion current commitHash
cd pharo-core
git fetch origin refs/pull/$PR/head git checkout -b pr$PR-local FETCH_HEAD git checkout $COMMIT git checkout -b pr$PR-merged git merge --no-edit pr$PR-local #git diff $COMMIT...pr$PR-merged > pr$PR.diff
Sorry I haven't the time to test this, but maybe it would do the same...?? git fetch origin refs/pull/$PR/merge:pr$PR-merged #git diff $COMMIT...pr$merged
https://coderwall.com/p/z5rkga/github-checkout-a-pull-request-as-a-branch
cheers -ben
ok, I'll try. thanks -- Pavel
Set the pull request name to the variable PR and the image commmit hash to the variable named COMMIT. It can be obtained by "SystemVersion current commitHash". As the result you are in a merged branch created for this pull requests (e.g. pr122-merged) and you need to reload all packages. It can be done from Iceberg where you will need to confirm several dialogs, or by this script:
repo := MCFileTreeRepository new directory: './pharo-core/src' asFileReference; yourself.
versions := OrderedCollection new. repo allFileNames do: [ :packageName | | wc | wc := MCWorkingCopy allManagers detect: [ :each | each packageName = (packageName withoutSuffix: '.package') ] ifNone: [nil]. wc ifNotNil: [ [(repo loadVersionFromFileNamed: packageName) load] on: MCMergeOrLoadWarning do: [:w | w resume ] ]] displayingProgress: [:e | e ].
The other option is to let display the classical Monticello merging window. For that you can use this script:
repo := MCFileTreeRepository new directory: './pharo-core/src' asFileReference; yourself.
versions := OrderedCollection new. repo allFileNames do: [ :packageName | | wc | wc := MCWorkingCopy allManagers detect: [ :each | each packageName = (packageName withoutSuffix: '.package') ] ifNone: [nil]. wc ifNotNil: [ versions add: (repo loadVersionFromFileNamed: packageName) ]] displayingProgress: [:e | e ].
merger := MCVersionMerger new. versions do: [:each | merger addVersion: each ]. merger mergeWithNameLike: 'merged'.
merger gatherChanges.
(merger instVarNamed: #records) do: [ :each | each mergePatch operations do: [:operation | operation chooseRemote.]].
(merger instVarNamed: #merger) operations do: #chooseRemote.
merger resolveConflicts.
However I have seen cases where for example the PR deleted some class and the MC merging tool was not able to see this change. So be careful with this approach. We need to look at that.
Do not look on this mail as something final. It's only a hint for brave sprinters. To find a good workflow how properly review our PR should be one of our priorities.
Cheers, -- Pavel