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

[Pharo 7.0-dev] Build #1234: 22420 Cleanup DateAndTimeLeapTest

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

There is a new Pharo build available! The status of the build #1234 was: SUCCESS. The Pull Request #1771 was integrated: "22420 Cleanup DateAndTimeLeapTest" Pull request url: https://github.com/pharo-project/pharo/pull/1771 Issue Url: https://pharo.fogbugz.com/f/cases/22420 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Sept. 7, 2018

[Pharo 7.0-dev] Build #1233: 22429-Enable-completion-for-the-selector-of-a-method

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

There is a new Pharo build available! The status of the build #1233 was: SUCCESS. The Pull Request #1779 was integrated: "22429-Enable-completion-for-the-selector-of-a-method" Pull request url: https://github.com/pharo-project/pharo/pull/1779 Issue Url: https://pharo.fogbugz.com/f/cases/22429 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Sept. 7, 2018

Re: [Pharo-dev] Bloc consideration - TextActions

by Ben Coman

On Fri, 7 Sep 2018 at 14:52, Ben Coman <btc(a)openinworld.com> wrote: > This is may not be worth investigating much in Morphic, but tips are > welcome. > Mainly this is a use case for consideration with the TextAction equivalent > in Bloc. > > For the #text: of a TextEntryDialogWindow > I wanted clickable link similar to the bottom of ```WelcomeHelp open``` > which would open a link in a web browser using > https://github.com/seandenigris/Pharo-Web-Browser. > > Experimenting in Playground, this[1] looked promising... > ``` > linkText := 'https://exercism.io/my/settings' asText > addAttribute: (TextAction new actOnClickBlock: [ > self inform: 'STUB: Invoke web browser here']); > addAttribute: (TextColor new color: Color cyan). > requestText := 'Enter your Exercism token (from ' asText, linkText, '):'. > requestText inspect. > ``` > where clicking on the link displays the information message. > > But this[2] didn't work... > ```UIManager default request: requestText``` > or to be more explicit, diving in a few calls down and simplifying, > neither did this... > ```(TextEntryDialogWindow new text: requestText) openInWorld``` > > I tracked it down to [2] having a locked TextMorph for requestText. > (observed using the following instrumentation...) > TextMorph isLocked > (text asString includesSubstring: 'Enter your Exercism token') > ifTrue: [ Transcript show: (super isLocked ifTrue: 'x' ifFalse:'.')]. > ^ super isLocked > and confirmed with this experiment... > ``` > tm1 := TextMorph new newContents: requestText. > tm2 := tm1 copy lock. > > tm1 openInWorld. > "Test clicking the text - action is invoked" > tm1 delete. > > tm2 openInWorld. > "Test clicking the text - action not invoked" > tm2 delete. > ``` > > This is due to #isLocked call here...: > ``` > Morph>>rejectsEvent: anEvent > "Return true to reject the given event. > Rejecting an event means neither the receiver nor any of it's submorphs > will be given any chance to handle it. > If the event is a mouse wheel event then only reject if the receiver is > not visible." > (anEvent isMouse and: [anEvent isMouseWheel]) > ifTrue: [^self visible not]. > ^self isLocked or: [self visible not] > ``` > > Now the functional paradox here is that: > * an unlocked TextMorph is editable, which you don't want for a static UI > label; > * but a locked TextMorph doesn't pass events through to trigger the > TextAction. > Thus it you can't have a clickable link in a static UI label. > Yet that is almost an essential feature in today's "linked" world > and I hope Bloc will provide a facility for actions within static/locked > text. > > cheers -ben > > P.S. Any tips on getting this to work in Pharo 6.1 would be appreciated. > Considering... TextMorph >> enabled ^super enabled and: [ self isLocked not ] a reasonable workaround might be... TextMorph subclass: #StaticTextMorph instanceVariableNames: '' classVariableNames: '' package: 'ExercismTools' StaticTextMorph >> enabled ^false then... stm1 := StaticTextMorph new newContents: requestText. stm1 openInWorld allows the link to be clicked while preventing edits. P.S. btw "TextMorph >> enabled:" seems suspect, since its visual only and doesn't prevent editing (if I guess correctly the expected functionality of enabled)

Sept. 7, 2018

[Pharo 7.0-dev] Build #1232: 22410-NECWorkspaceController-is-not-used

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

There is a new Pharo build available! The status of the build #1232 was: FAILURE. The Pull Request #1778 was integrated: "22410-NECWorkspaceController-is-not-used" Pull request url: https://github.com/pharo-project/pharo/pull/1778 Issue Url: https://pharo.fogbugz.com/f/cases/22410 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

Sept. 7, 2018

Re: [Pharo-dev] Release test for unused temps and other

by Guillermo Polito

Hi, I don't think that 20 seconds will affect the overall process :). It will only become a problem if we increment the number of these kind of tests by *a lot*... Another solution is to make some tests run in parallel. But for this to run properly we have to increase the number of slaves in Jenkins, and the bad news is that we reached the current maximum limit (of 5 slaves). We have to talk to the inria people to see what solution they could provide to us for this, because otherwise the infrastructure does not scale for lots of PRs with big jobs. Guille On Fri, Sep 7, 2018 at 8:23 AM Alistair Grant <akgrant0710(a)gmail.com> wrote: > On Thu, 6 Sep 2018 at 23:06, Cyril Ferlicot D. <cyril.ferlicot(a)gmail.com> > wrote: > > > > > > Maybe we could have a package with release tests that are not executed > > by the CI but that are executed at the moment of the feature freeze and > > some time before the Pharo release to ensure the quality of the code > > base for tests that are too long? > > The drawback of this approach is that it relies on the original author > going back and submitting a fix in a timely manner (quite possibly > after they've moved on to other things). > > If the tests could be run on a specified set of classes / methods, > perhaps we could run them as part of Iceberg, i.e. when going to > commit code, these tests are run on the classes in the commit and a > warning opened prompting the user to fix them before actually > committing. > > Cheers, > Alistair > > -- Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - *http://www.cnrs.fr <http://www.cnrs.fr>* *Web:* *http://guillep.github.io* <http://guillep.github.io> *Phone: *+33 06 52 70 66 13

Sept. 7, 2018

Bloc consideration - TextActions

by Ben Coman

This is may not be worth investigating much in Morphic, but tips are welcome. Mainly this is a use case for consideration with the TextAction equivalent in Bloc. For the #text: of a TextEntryDialogWindow I wanted clickable link similar to the bottom of ```WelcomeHelp open``` which would open a link in a web browser using https://github.com/seandenigris/Pharo-Web-Browser. Experimenting in Playground, this[1] looked promising... ``` linkText := 'https://exercism.io/my/settings' asText addAttribute: (TextAction new actOnClickBlock: [ self inform: 'STUB: Invoke web browser here']); addAttribute: (TextColor new color: Color cyan). requestText := 'Enter your Exercism token (from ' asText, linkText, '):'. requestText inspect. ``` where clicking on the link displays the information message. But this[2] didn't work... ```UIManager default request: requestText``` or to be more explicit, diving in a few calls down and simplifying, neither did this... ```(TextEntryDialogWindow new text: requestText) openInWorld``` I tracked it down to [2] having a locked TextMorph for requestText. (observed using the following instrumentation...) TextMorph isLocked (text asString includesSubstring: 'Enter your Exercism token') ifTrue: [ Transcript show: (super isLocked ifTrue: 'x' ifFalse:'.')]. ^ super isLocked and confirmed with this experiment... ``` tm1 := TextMorph new newContents: requestText. tm2 := tm1 copy lock. tm1 openInWorld. "Test clicking the text - action is invoked" tm1 delete. tm2 openInWorld. "Test clicking the text - action not invoked" tm2 delete. ``` This is due to #isLocked call here...: ``` Morph>>rejectsEvent: anEvent "Return true to reject the given event. Rejecting an event means neither the receiver nor any of it's submorphs will be given any chance to handle it. If the event is a mouse wheel event then only reject if the receiver is not visible." (anEvent isMouse and: [anEvent isMouseWheel]) ifTrue: [^self visible not]. ^self isLocked or: [self visible not] ``` Now the functional paradox here is that: * an unlocked TextMorph is editable, which you don't want for a static UI label; * but a locked TextMorph doesn't pass events through to trigger the TextAction. Thus it you can't have a clickable link in a static UI label. Yet that is almost an essential feature in today's "linked" world and I hope Bloc will provide a facility for actions within static/locked text. cheers -ben P.S. Any tips on getting this to work in Pharo 6.1 would be appreciated.

Sept. 7, 2018

Re: [Pharo-dev] Release test for unused temps and other

by Alistair Grant

On Thu, 6 Sep 2018 at 23:06, Cyril Ferlicot D. <cyril.ferlicot(a)gmail.com> wrote: > > > Maybe we could have a package with release tests that are not executed > by the CI but that are executed at the moment of the feature freeze and > some time before the Pharo release to ensure the quality of the code > base for tests that are too long? The drawback of this approach is that it relies on the original author going back and submitting a fix in a timely manner (quite possibly after they've moved on to other things). If the tests could be run on a specified set of classes / methods, perhaps we could run them as part of Iceberg, i.e. when going to commit code, these tests are run on the classes in the commit and a warning opened prompting the user to fix them before actually committing. Cheers, Alistair

Sept. 7, 2018

Iceberg > review pull request & repo name clash

by Ben Coman

Two concerns from one flow of events... I was trying out "review a pull request" feature for the first time. This looks very promising, but I got stuck. In Pharo 6.1... Downloaded http://files.pharo.org/platform/Pharo6.1-win.zip Updated Icerberg per https://github.com/pharo-vcs/iceberg#update-iceberg Iceberg > Settings > Use custom keys = Enabled > Add > Clone remote repository Remote URL = git@github.com:exercism/pharo.git ==> "pharo" repo appears, status "Not loaded" > "pharo" repo > Packages ==> BaselineOfExercism, Exercism, etc > "pharo" repo > Github > View pull request > origin > enter github account password ==> shows PR#98 "Move setup to class initialization..." > right-click 98 > Review pull request ==> an empty window titled "Browsing pull request: #98...." appeared (see attached) Trying the same in Pharo 7 gave a different problem... >From Launcher started Pharo-7.0.0-alpha.build.1231.sha.cf8aa48.arch.32bit... Iceberg > Settings > Use custom keys = Enabled > "pharo" repo > Forget Repository > Ticked "Also remove repository from file system" > Add > Clone remote repository Remote URL = git@github.com:exercism/pharo.git ==> "pharo" appears, status = "Fetch required. Unknown cf8aa48" > "pharo" repo > Fetch ==> no change in status, but I do see the the expected files in repo on disk > "pharo" repo > Packages ==> only shows LICENSE and README.md. Why did this "just work" in 6.1 ? It seems the source directory is wrong. From this point is it possible to update the source directory to "dev/src/" ? Maybe its due to the name clash "pharo-project/pharo" versus "exercism/pharo" and something was retained after the "Forget Repository". Such a name clash is unfortunate, but is a corner case that should be handled. This could be a useful test case. Before I log an issue, could someone check I'm not doing something wrong. cheers -ben P.S. A work around might have been giving a custom local name for the exercism/pharo repo, much like you can specify the folder created by command line 'git clone'. Is such a feature planned for Iceberg?

Sept. 7, 2018

Re: [Pharo-dev] Release test for unused temps and other

by Gabriel Cotelli

In my experience this is not a good idea. When you're in the pressure of the release the last thing you want it's to fix things that shouldn't be merged in that form. It sends the wrong message to the people contributing, If you have this fail in your own PR you can easily fix it and learn for it for the next time. But at the time Pharo enter feature freeze probably is someone else who has to fix it. Maintaining code quality is way easier if you keep it in good shape everyday. > Hi, > > Maybe we could have a package with release tests that are not executed > by the CI but that are executed at the moment of the feature freeze and > some time before the Pharo release to ensure the quality of the code > base for tests that are too long? > > > Thanks > > T. > > > > > > > > > > > -- > Cyril Ferlicot > https://ferlicot.fr > >

Sept. 6, 2018

Re: [Pharo-dev] Release test for unused temps and other

by Cyril Ferlicot D.

Le 06/09/2018 Ã 17:50, Torsten Bergmann a Ã©critÂ : > Hi, > > the very nice contribution on GIFReadWriter/animated images (see https://pharo.manuscript.com/f/cases/22137) > unfortunately introduced some methods that had no sender, were uncategorized and had unused temporary variables, etc. > > Something that is meanwhile shown by our tools and easy to fix. I guess it was some overseen leftover. Unfortunately this > also slipped through the integration process as it was merged without an approval from a community reviewer. > > I dont want to lament too much on this specifc case (especially as I now fixed it in https://pharo.fogbugz.com/f/cases/22426) > > But after I cleaned the P7 image this spring to not have unused temporaries anymore I was suprised to find such > glitches again now. So to me it shows that we need to write more tests to detect violations and keep our achieved quality standards > up and also should not solely rely on the review process. > > Otherwise we need to invest our time in cleaning up our own stuff afterwards again and again. > > To be specific one can evaluate: > > CompiledMethod allInstances select: [:m | m ast temporaries anySatisfy: [:x | x binding isUnused ]]. > > to find unused temps - which would be easy to package up into one of the release test to ensure no unused temp vars are left > (the tearDown would require an "ASTCache reset"). > > The problem is that this simple test would take around 20-30 seconds depending on machine and I do not know if it would be a > good idea/good solution to integrate. Especially I'm not sure if it is worth the issue or will kill the CI again and again. > > What do you think about it? Comments appreciated? > Hi, Maybe we could have a package with release tests that are not executed by the CI but that are executed at the moment of the feature freeze and some time before the Pharo release to ensure the quality of the code base for tests that are too long? > Thanks > T. > > > > -- Cyril Ferlicot https://ferlicot.fr

Sept. 6, 2018