2017-06-30 17:58 GMT+02:00 Ben Coman <b...@openinworld.com>: > > > On Fri, Jun 30, 2017 at 4:28 PM, Pavel Krivanek <pavel.kriva...@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 >> > >
