January 2018 - Pharo-dev - lists.pharo.org

Re: [Pharo-dev] Iceberg merged pull requests?

by Bernhard Pieber

Hi Esteban, Thanks for your answer. See more questions below. > Am 04.01.2018 um 09:16 schrieb Esteban Lorenzano <estebanlm(a)gmail.com>: > >> On 3 Jan 2018, at 23:09, Stephane Ducasse <stepharo.self(a)gmail.com> wrote: >> >> You should double click on the pharo lines and looks in the update >> pane you will see the updates (but you should pull from pharo to sync >> your repo). >> >> Stef >> >> On Tue, Jan 2, 2018 at 10:45 PM, Bernhard Pieber <bernhard(a)pieber.com> wrote: >>> I saw that two pull requests were accepted and merged into the development branch. I had expected that Iceberg would show me them somewhere. However, in the branches pane of my repository the status pharo-project/development is shown as Up to date. >>> >>> Shouldn't there be some indication that new code is available to be merged, or am I misunderstanding Iceberg? > > you are misunderstanding iceberg :) That's what I suspected. ;-) > your repository shows the status of the image against your local copy of the repository, not the remote one. > If you want to see those changes, you need to first âfetchâ the remote repository (which will update the commits available in the local one). > this is a task (fetching) that other tools do in background time to time (for example, sourcetree does it like when you focus on the window), to show this as a âlive experienceâ, but we cannot do the same because a call to fetch would block the image (since FFI is blocking for the moment), then you will need to do that by hand until we get threaded FFI. Is this fetching from the upstream pharo-project repository something I should be able to do in Iceberg? I had assumed it is, but I could not find it? In SourceTree there is a command called Refresh Remote Status. Is there something akin to that in Iceberg? > also, notice that updating Pharo from iceberg is a very bad idea, and you can have an unusable image after that (update other projects will usually work). I am afraid I still don't understand. I thought that was the point of Iceberg? If not, what is the suggested workflow for merged pull requests to the pharo-project upstream? Should I build a new development image whenever new pull requests are merged? Bernhard

Jan. 6, 2018

Tonel trouble in P7 development branch for collection tests

by Torsten Bergmann

Hi, when contributing to Pharo 7 sometimes you might have noticed that during commit Iceberg will show you TDictionaryIncludesWithIdentityCheckTest>>nonEmptyWithCopyNonIdentical. TDictionaryTest>>nonEmptyWithCopyNonIdentical. as a deleted method. Maybe I was the only one seeing the effect so far - but I wondered and checked. Reason is that in the #development branch the src/Collections-Tests/TDictionaryIncludesWithIdentityCheckTest.trait.st src/Collections-Tests/DictionaryTest.class.st files are broken for Tonel format for the method #nonEmptyWithCopyNonIdentical as there was an additional "dot" after the selector. Here is the diff: https://github.com/pharo-project/pharo/pull/651/files I opened case 20932 for that topic and fixed it with the PR 651. Hope this will solve the issue once merged. Also I have not checked where this originally came from to find the root cause of the trouble. Bye T.

Jan. 6, 2018

Re: [Pharo-dev] extract method refactoring with assignment

by John Brant

> On Jan 5, 2018, at 11:10 AM, Eliot Miranda <eliot.miranda(a)gmail.com> wrote: > > Hi All, > > I tried an extract to method refactoring yesterday that failed. The method looked like > > | ... v1 v2 v3 | > .... > v1 := self m1. > v2 := self m2. > v3 := v3 collect: [:e| ...]. > .... > > such that v1 and v2 were only used in the block, and the block was complex enough and did something useful e Pugh that I wanted to refactor as > > > | ... v1 v2 v3 | > .... > v3 := self complexCollect: v3. > .... > > complexCollect: a > | v1 v2 | > v1 := self m1. > v2 := self m2. > ^a collect: [:e| ...] > > So I was forced by the fact that assignment is written with the variable on the LHS to include "v3 := " in the selection to extract to method, even though my intent was for it not to be included. And of course the refactoring attempt fails precisely because the assignment /is/ included :-) > > Shouldn't this case be specially handled do that and trailing assignment is left behind in the refactored original method? Ifso, how would one go about it? > > Note that part of the prerequisite checks should also check that the variables v1 & v2 are not used elsewhere in the origins nail method and whether the refactoring is smart enough to remove them from the refactored origins nap is optional but definitely preferred. > When you extract multiple statements and the last one is an assignment, it should ask if you want to extract the assignment also. Depending on what you select the analysis is a bit different. For example, consider this method: foo | a b | a := self printString. âStatement 1" b := 1. âStatement 2" a := a âStatement 3" inject: 0 into: [ :s :e | b := b + 1. s + b + e asInteger ]. ^ a âStatement 4" If you select statements 2 & 3 and do an extract, it should ask if you want to extract the assignment. If you donât, then you get something like what you wanted above: foo | a | a := self printString. a := self bar: a. ^ a bar: a | b | b := 1. ^ a inject: 0 into: [ :s :e | b := b + 1. s + b + e asInteger ] If you do extract the assignment, it isn't allowed since we are splitting the âaâ variable (one in #foo and one in #bar). With a little bit of analysis we might be able to determine that the âaâ in #foo isnât live when we are running #bar, but if âaâ was used in a block, then things get really tough for that type of analysis. If you do this in VW, you get an error message dialog saying what the problem is. However, if you run in Pharo, you get a notice that gets displayed rather quickly and the refactoring proceeds like everything is fine even though it is producing code that will not run. If you extract statements 1-3 without the assignment, it sees that you are assigning âaâ inside the extracted method and using it in the unextracted code, and also you are needing to return the result of the #inject:into:. Since it appears that we need to return two items, it throws an error. In VW, this error causes a dialog to appear with what is wrong, and the refactoring aborts. In Pharo, the information quickly appears and the refactoring continues (which throws an unexpected error that we are trying to add another return statement after an existing return statement). In this case we could do some analysis to see that we donât need to return the âaâ variable since it is immediately assigned. Finally, if you extract statements 1-3 with the assignment, you should get: foo | a | a := self bar. ^ a bar | a b | a := self printString. b := 1. a := a inject: 0 into: [ :s :e | b := b + 1. s + b + e asInteger ]. ^ a In this case, you didnât need to return the value of #inject:into: so it could return the âaâ variable as the single return value from the extracted method. Anyway, Pharo should be fixed to not resume errors that the refactorings are throwing. There is a difference between error and warning. John Brant

Jan. 5, 2018

extract method refactoring with assignment

by Eliot Miranda

Hi All, I tried an extract to method refactoring yesterday that failed. The method looked like | ... v1 v2 v3 | .... v1 := self m1. v2 := self m2. v3 := v3 collect: [:e| ...]. .... such that v1 and v2 were only used in the block, and the block was complex enough and did something useful e Pugh that I wanted to refactor as | ... v1 v2 v3 | .... v3 := self complexCollect: v3. .... complexCollect: a | v1 v2 | v1 := self m1. v2 := self m2. ^a collect: [:e| ...] So I was forced by the fact that assignment is written with the variable on the LHS to include "v3 := " in the selection to extract to method, even though my intent was for it not to be included. And of course the refactoring attempt fails precisely because the assignment /is/ included :-) Shouldn't this case be specially handled do that and trailing assignment is left behind in the refactored original method? Ifso, how would one go about it? Note that part of the prerequisite checks should also check that the variables v1 & v2 are not used elsewhere in the origins nail method and whether the refactoring is smart enough to remove them from the refactored origins nap is optional but definitely preferred. _,,,^..^,,,_ (phone)

Jan. 5, 2018

[Pharo 7.0-dev] Build #411: 20925 Super setUp need to be called in UnifiedFFI tests

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #411 was: FAILURE. The Pull Request #645 was integrated: "20925 Super setUp need to be called in UnifiedFFI tests" Pull request url: https://github.com/pharo-project/pharo/pull/645 Issue Url: https://pharo.fogbugz.com/f/cases/20925 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Jan. 5, 2018

[Pharo 7.0-dev] Build #410: 20926-Super-setUp-need-to-be-called-in-MCWorkingCopyBrowserTest

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #410 was: SUCCESS. The Pull Request #646 was integrated: "20926-Super-setUp-need-to-be-called-in-MCWorkingCopyBrowserTest" Pull request url: https://github.com/pharo-project/pharo/pull/646 Issue Url: https://pharo.fogbugz.com/f/cases/20926 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Jan. 5, 2018

[Pharo 7.0-dev] Build #409: 20924 Fix unused var tutorial in TutorialPlayerTestWithMockTutorial>>setUp

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #409 was: SUCCESS. The Pull Request #644 was integrated: "20924 Fix unused var tutorial in TutorialPlayerTestWithMockTutorial>>setUp" Pull request url: https://github.com/pharo-project/pharo/pull/644 Issue Url: https://pharo.fogbugz.com/f/cases/20924 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Jan. 5, 2018

Re: [Pharo-dev] gtInspector+FastTable enhancement requests

by Ben Coman

On 5 January 2018 at 15:38, Ben Coman <btc(a)openinworld.com> wrote: > > > On 10 December 2017 at 16:51, Ben Coman <btc(a)openinworld.com> wrote: > >> Feedback for GT'ers... >> >> I'm probably ab-using FastTable support in gtInspector, >> but as a quick hack during initial prototyping it was *very* nice >> to be able review retrieved data as a table of instances >> with a column per instance variable. >> >> Some enhancements that would make this even better... >> 1. Horizontal scroll bar - There are columns past the right margin that I >> can't get to for lack of one. >> > > I'm having a go at this. The easy part adding the scrollbar morph is done > and before proceeding further I'm seeking advice on the recommended > approach > to translating the columns. > > I've uploaded Morphic-Widgets-FastTable-BenComan.223 to Pharo6InBox. > Could someone familiar with FastTable internals review this on top of 60528 > and advise the next step I should take. > I should have mentioned, run... FTExample>>exampleTableHorizontalScroll to see demo of what I've done so far. btw, to make it safer to trial this horizontal scroll bar UI code all modifications are guarded by... trialHSB ifNil: [ ] ifNotNil: [] cheers -ben

Jan. 5, 2018

Re: [Pharo-dev] gtInspector+FastTable enhancement requests

by Ben Coman

On 10 December 2017 at 16:51, Ben Coman <btc(a)openinworld.com> wrote: > Feedback for GT'ers... > > I'm probably ab-using FastTable support in gtInspector, > but as a quick hack during initial prototyping it was *very* nice > to be able review retrieved data as a table of instances > with a column per instance variable. > > Some enhancements that would make this even better... > 1. Horizontal scroll bar - There are columns past the right margin that I > can't get to for lack of one. > I'm having a go at this. The easy part adding the scrollbar morph is done and before proceeding further I'm seeking advice on the recommended approach to translating the columns. For what I've done so far, I was going to submit for review via the latest Pharo 7 Contribution process, but I hit a blocker... http://forum.world.st/IMPORTANT-PHARO-CONTRIBUTION-PROCESS-UPDATED-PLEASE-R… so I've uploaded Morphic-Widgets-FastTable-BenComan.223 to Pharo6InBox. Could someone familiar with FastTable internals review this on top of 60528 and advise the next step I should take. cheers -ben P.S. I expect to integrate this in Pharo 7 rather than Pharo 6, but this moment its easier for me to ask for help this way.

Jan. 5, 2018

Re: [Pharo-dev] [IMPORTANT] PHARO CONTRIBUTION PROCESS UPDATED! PLEASE READ!

by Ben Coman

On 14 December 2017 at 20:19, Esteban Lorenzano <estebanlm(a)gmail.com> wrote: > Hi! > > Iâm working on simplifying the contribution process, after collecting > opinions/experiences last couple of months. > As you know, Pharo contribution process is still WIP and we aim to have it > as smooth as possible for Pharo 7.0 release. Now, after observe the idea of > the âsystem repositoriesâ was a bad idea because it introduced extra and > non standard âpathâ to contribution, I managed to remove that to > reestablish âthe regular wayâ: you will now need to add pharo repository > just as any other repository you add, by cloning or adding local repository. > > I took Guilleâs doc and moved it to pharo project (it does not has sense > to have it living in a contributorâs repository when is so important). You > can find it here: > > https://github.com/pharo-project/pharo/wiki/Contribute-a-fix-to-Pharo > > This document is also updated to reveal this new process, please read it. > > How to update your startup scripts? > Some people has added startup scripts to easy the first part of > contribution. Instead enabling system repositories, etc. you now need to > replace that with this: > > (IceRepositoryCreator new > location: '/path/to/pharo-project/pharo' asFileReference; > subdirectory: 'src'; > createRepository) > register > > PLEASE, PLEASE, PLEASEâ¦ take a moment to read and try the document. Is > very important that document reflects new process and works reliable in > different scenarios (I validated it on macOS and Windows, and assumed it > worked fine on linux but you knowâ¦ bad assumptions is the base of failure > ;) ) > > Iâm eager to hear your feedback and continue enhancing the process. > > (yes, Stef, I know UI is still cumbersomeâ¦ Iâm working on that :) ) > > cheers! > Esteban > Using https://github.com/pharo-project/pharo/wiki/Contribute-a-fix-to-Pharo with my pharo branch "22 commits behind pharo-project:master" https://github.com/bencoman/pharo/tree/master >From PharoLauncher, I created fresh latest image from Pharo-7.0.0-alpha.build.408.sha.5369e34.arch.64bit (btw, download took about 30 seconds which was reasonably good) Opened image WorldMenu > Iceberg Iceberg > Clone Repository Remote URL = git@github.com:bencoman/pharo.git Local Directory (default) = /home/ben/Pharo/images/FastTable-Pharo-7.0.0-alpha.build.402.sha.d5d4c99.arch.64bit-01/pharo-local/iceberg Code subdirectory = src <Create Repository> A bit disconcerting that for about 1.5 minutes the UI is frozen, but eventually "pharo" entry appears in IceBerg with... Name = *pharo Current branch = master Loaded version = fc92bbe (7 July 2017) Status = uncommitted changes Right-clicked on *pharo > Synchronise repository up popped dialog "Synchronising" on tab [Commit your changes] with about 800 changes with all items ticked. (btw, I clicked <Refresh Tree> all the changes are unticked, and only the three toplevel items GT-Spotter, Kernel & Reflectivity are ticked. Is that ideal? What is the typical use case of that button. Anyway, I close the dialog, and again right-click on *pharo > Synchronise repository) (btw, reviewing change here is nice.) A bit disconcerting that buttons <Commit onto master> and <Commit and push onto master> are both disabled so there is no action I can do from this default, so I proceed to [Push] tab. I'm feel unsure whether there should be some item listed under "New local commits to be published" (there are none) but pushing <Publish all> seems the only thing possible, so I do that. That seems to complete after 10 seconds, but still status = "Uncommitted changes" in the main Iceberg window. Opening a new IceBerg window shows the same thing. So what more do I need to do to get my Pharo repo "Up to date" ? cheers -ben

Jan. 5, 2018