On Fri, Jun 30, 2017 at 4:28 PM, Pavel Krivanek <[email protected]> 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? > > 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 > 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 >
