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

Re: [Pharo-dev] ***important*** cannot remove a class anymore...

by stepharo

Le 6/9/15 00:33, Sven Van Caekenberghe a Ã©crit : >> On 05 Sep 2015, at 21:34, stepharo <stepharo(a)free.fr> wrote: >> >> >> >> Le 5/9/15 19:18, Yuriy Tymchuk a Ã©crit : >>> In fact you couldnât spot the issue by just looking at the patch :) there was a very strange thing that could be fixed with one line, but to identify it Iâve spent probably 1.5 hours :) >> sure this is not my point. >> my point is that we need to be able to know what changed. Right now I see a title and I do not have the time to click on each github link. >> I want to see diffs right in front of my nose. > I know very little about git, but the first thing that I tried worked: > > git clone https://github.com/pharo-project/pharo-core.git > cd pharo-core/ > git show 2536752ecdd88e15b00c6d3e29cb055d108731c8 I went to github. I do not want to have to type the hash and this. How do I know that this this hash corresponds to the fixes that have been submitted? Then the output is ***awful***. So if this is that easy why can we have a mail with the diff? Stef > > I assume that we could put that textual information in the mail. > > In my terminal, it is coloured, but I still think that the github page is much better: > > https://github.com/pharo-project/pharo-core/commit/2536752ecdd88e15b00c6d3e… > > (It happens that this commit changed quite a lot) > >> Stef >>> Uko >>> >>>> On 05 Sep 2015, at 19:03, Nicolai Hess <nicolaihess(a)web.de> wrote: >>>> >>>> >>>> >>>> 2015-09-05 11:48 GMT+02:00 Marcus Denker <marcus.denker(a)inria.fr>: >>>> >>>>> On 05 Sep 2015, at 11:40, stepharo <stepharo(a)free.fr> wrote: >>>>> >>>>> How can I see the changes? >>>>> Our process is not good. Most of us do not get any chance understanding what is changing. >>>>> >>>> -> download the image before it was added >>>> -> merge the slice. >>>> >>>> Yes, our process is not goodâ¦ but from a review perspective, this issue is the best we can do. >>>> *two* reviews, both from people actively contributing to exactly that part of the system. >>>> >>>> If we require more, we will be back at a process where due to Fear we do nothing. >>>> >>>> >>>> Nautilus is a difficult beast, you all know. Even if you look close at the merge diff you may >>>> miss some important relation that only really visible during debugging. >>>> I did some test when reviewing this fix. But really, there are so many >>>> ways do you need to consider >>>> - different ways for opening nautilus >>>> - navigate with keys and mouse >>>> - selecting updating / changes from other browser and system changes. >>>> - refactorings >>>> >>>> It is really not that easy and I am happy that Franck and Yuri are actually trying to >>>> improve Nautilus. >>>> you can not easily tests all and make the changes and reviews bullet proof. >>>> >>>> >>>> nicolai >>>> >>>> >>>> >>>>>> Hi, >>>>>> >>>>>> This is a side effect of >>>>>> >>>>>> https://pharo.fogbugz.com/f/cases/16475/Nautilus-sends-too-many-announcemen… >>>>>> >>>>>> (which was reviewed by two people, so not obvious). >>>>>> >>>>>> What happened is that #updatePackageGroupAndClassList calls itself via #selectedClass: leading to a loop. >>>>>> >>>>>> Should be easy to fix for the people involved in case 16475. >>>>>> >>>>>> Marcus >>>>>> >>>>>> >>>>>> On Sat, Sep 5, 2015 at 8:51 AM, stepharo <stepharo(a)free.fr> wrote: >>>>>> Hi >>>>>> >>>>>> I do not know if this is linked to recent changes but we cannot remove classes or move them to another package. >>>>>> >>>>>> Stef >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> -- >>>>>> Marcus Denker -- denker(a)acm.org >>>>>> http://www.marcusdenker.de > >

Sept. 6, 2015

[pharo-project/pharo-core]

by GitHub

Branch: refs/tags/50307 Home: https://github.com/pharo-project/pharo-core

Sept. 6, 2015

[pharo-project/pharo-core] 96f3ce: 50307

by GitHub

Branch: refs/heads/5.0 Home: https://github.com/pharo-project/pharo-core Commit: 96f3ceffe16fa3c72e2ba9e7c7590e00118c4d2f https://github.com/pharo-project/pharo-core/commit/96f3ceffe16fa3c72e2ba9e7… Author: Jenkins Build Server <board(a)pharo-project.org> Date: 2015-09-06 (Sun, 06 Sep 2015) Changed paths: M AST-Core.package/RBParser.class/instance/accessing/parseExpression_.st M AST-Core.package/RBParser.class/instance/accessing/parseMethod_.st M AST-Core.package/RBParser.class/instance/private-parsing/parseBlock.st A AST-Core.package/RBParser.class/instance/private-parsing/parseIncompleteExpression_.st M AST-Core.package/RBParser.class/instance/private-parsing/parseVariableNode.st M AST-Core.package/RBScanner.class/instance/accessing/scanToken.st M AST-Core.package/RBScanner.class/instance/private-scanning/scanLiteral.st M AST-Core.package/RBScanner.class/instance/private-scanning/scanLiteralString.st M AST-Core.package/RBScanner.class/instance/private-scanning/stripComment.st M Morphic-Base.package/EmbeddedMenuMorph.class/instance/keyboard control/moveSelectionDown_event_.st M Morphic-Base.package/MenuItemMorph.class/instance/accessing/hasIconOrMarker.st M Morphic-Base.package/MenuItemMorph.class/instance/accessing/hasMarker.st M Morphic-Base.package/MenuItemMorph.class/instance/events/themeChanged.st M Morphic-Base.package/MenuItemMorph.class/instance/grabbing/aboutToBeGrabbedBy_.st M Morphic-Base.package/MenuItemMorph.class/instance/meta actions/wantsHaloFromClick.st M Morphic-Base.package/MenuItemMorph.class/instance/nil/deselectItem.st A Morphic-Base.package/MenuItemMorph.class/instance/testing/isMenuItemMorph.st A Morphic-Base.package/MenuLineMorph.class/instance/testing/isMenuLineMorph.st R Morphic-Base.package/MenuMorph.class/instance/accessing/hasItems.st R Morphic-Base.package/MenuMorph.class/instance/accessing/hasSubMenu_.st M Morphic-Base.package/MenuMorph.class/instance/accessing/items.st M Morphic-Base.package/MenuMorph.class/instance/accessing/lastItem.st M Morphic-Base.package/MenuMorph.class/instance/accessing/menuItems.st M Morphic-Base.package/MenuMorph.class/instance/accessing/popUpOwner.st M Morphic-Base.package/MenuMorph.class/instance/actions/deleteIfPopUp.st A Morphic-Base.package/MenuMorph.class/instance/construction-UIManager/addItem.st A Morphic-Base.package/MenuMorph.class/instance/construction-UIManager/addTitle.st M Morphic-Base.package/MenuMorph.class/instance/construction-title/addStayUpIcons.st R Morphic-Base.package/MenuMorph.class/instance/construction-title/newTitleIconCalling_form_ballonText_.st A Morphic-Base.package/MenuMorph.class/instance/construction-title/newTitleIconCalling_form_helpText_.st M Morphic-Base.package/MenuMorph.class/instance/construction/addAllFrom_.st M Morphic-Base.package/MenuMorph.class/instance/construction/addLine.st M Morphic-Base.package/MenuMorph.class/instance/construction/addMenuItem_.st M Morphic-Base.package/MenuMorph.class/instance/construction/addToggle_target_selector_getStateSelector_enablementSelector_argumentList_.st M Morphic-Base.package/MenuMorph.class/instance/initialization/initialize.st M Morphic-Base.package/MenuMorph.class/instance/invoking/informUserAt_during_.st M Morphic-Base.package/MenuMorph.class/instance/keyboard control/displayFiltered_.st M Morphic-Base.package/MenuMorph.class/instance/keyboard control/moveSelectionDown_event_.st M Morphic-Base.package/MenuMorph.class/instance/menu/addCustomMenuItems_hand_.st R Morphic-Base.package/MenuMorph.class/instance/menu/addItem.st R Morphic-Base.package/MenuMorph.class/instance/menu/addTitle.st M Morphic-Base.package/MenuMorph.class/instance/menu/detachSubMenu_.st M Morphic-Base.package/MenuMorph.class/instance/menu/doButtonAction.st M Morphic-Base.package/MenuMorph.class/instance/menu/removeStayUpItems.st M Morphic-Base.package/MenuMorph.class/instance/menu/setTarget_.st M Morphic-Base.package/MenuMorph.class/instance/menu/target_.st A Morphic-Base.package/MenuMorph.class/instance/submorphs-add%2Fremove/addMorphBack_.st A Morphic-Base.package/MenuMorph.class/instance/testing/hasItems.st A Morphic-Base.package/MenuMorph.class/instance/testing/hasSubMenu_.st A Morphic-Base.package/MenuMorph.class/instance/testing/isMenuMorph.st A Morphic-Base.package/extension/Morph/instance/isMenuItemMorph.st A Morphic-Base.package/extension/Morph/instance/isMenuLineMorph.st A Morphic-Base.package/extension/Morph/instance/isMenuMorph.st M Morphic-Base.package/extension/WorldState/class/windowsOn_.st M Polymorph-Widgets.package/IdentifierChooserMorph.class/instance/event handling/selectNextItem.st M Polymorph-Widgets.package/IdentifierChooserMorph.class/instance/event handling/selectPreviousItem.st M Polymorph-Widgets.package/IdentifierChooserMorph.class/instance/event handling/switchToOtherColumn_.st M Polymorph-Widgets.package/IdentifierChooserMorph.class/instance/ui-building/newChoicesMorph.st M Polymorph-Widgets.package/IdentifierChooserMorph.class/instance/ui-building/newMenuWith_.st R ScriptLoader50.package/ScriptLoader.class/instance/pharo - scripts/script50306.st A ScriptLoader50.package/ScriptLoader.class/instance/pharo - scripts/script50307.st R ScriptLoader50.package/ScriptLoader.class/instance/pharo - updates/update50306.st A ScriptLoader50.package/ScriptLoader.class/instance/pharo - updates/update50307.st M ScriptLoader50.package/ScriptLoader.class/instance/public/commentForCurrentUpdate.st M Spec-MorphicAdapters.package/MorphicMenuAdapter.class/instance/private/buildWidgetPopup.st M Tool-FileList.package/extension/MenuMorph/instance/addServices_for_extraLines_.st Log Message: ----------- 50307 16483 AST-Core should not depend on System-Localization https://pharo.fogbugz.com/f/cases/16483 16472 Minor improvements to MenuMorph (2) https://pharo.fogbugz.com/f/cases/16472 16492 no syntax highlighting if last expression is invalid code https://pharo.fogbugz.com/f/cases/16492 http://files.pharo.org/image/50/50307.zip

Sept. 6, 2015

Re: [Pharo-dev] ***important*** cannot remove a class anymore...

by stepharo

Peter I just want to get an idea of what is happening. Even after commit. Le 6/9/15 01:41, Peter UhnÃ¡k a Ã©crit : > > git clone https://github.com/pharo-project/pharo-core.git > cd pharo-core/ > git show 2536752ecdd88e15b00c6d3e29cb055d108731c8 > > > But this is post-integration, we want to review things before they > break something, not after. > So ideally monkey reviews code on CI and if it passes the diff is sent > from CI to the mailing list. > > Also these commits combine several unrelated issues, which is not good > for review. > > Isn't what Squeak uses open-source and available somewhere? Can't we > just reuse it? > > Peter

Sept. 6, 2015

Re: [Pharo-dev] ***important*** cannot remove a class anymore...

by Sven Van Caekenberghe

> On 06 Sep 2015, at 01:41, Peter UhnÃ¡k <i.uhnak(a)gmail.com> wrote: > > git clone https://github.com/pharo-project/pharo-core.git > cd pharo-core/ > git show 2536752ecdd88e15b00c6d3e29cb055d108731c8 > > But this is post-integration, we want to review things before they break something, not after. > So ideally monkey reviews code on CI and if it passes the diff is sent from CI to the mailing list. > > Also these commits combine several unrelated issues, which is not good for review. > > Isn't what Squeak uses open-source and available somewhere? Can't we just reuse it? > > Peter You are right, Peter. But the way the rest of the world (and many people using git from Pharo or other Smalltalk implementations) is that you propose patches which I believe are quite similar to commits. That would be the equivalent of our slice, I guess. Sven

Sept. 6, 2015

Re: [Pharo-dev] when:send:to: vs. subscribe:send:to:

by Tudor Girba

Probably yes. However, in that case the issue is at least internal to the implementation as nobody will actually use that class directly. Cheers, Doru On Sat, Sep 5, 2015 at 11:56 PM, Bernardo Ezequiel Contreras < vonbecmann(a)gmail.com> wrote: > Excuse me, but what about the following messages > > MessageBrowser browse: > ({WeakSubscriptionBuilder>>#on:do:. > WeakSubscriptionBuilder>>#on:send:to:}) > > should be deprecated, or not? > > > > On Sat, Sep 5, 2015 at 5:24 PM, stepharo <stepharo(a)free.fr> wrote: > >> Hi >> >> in announcer we have >> >> when: anAnnouncementClass send: aSelector to: anObject >> "Declare that when anAnnouncementClass is raised, anObject should >> receive the message aSelector. >> When the message expects one argument (eg #fooAnnouncement:) the >> announcement is passed as argument. >> When the message expects two arguments (eg >> #fooAnnouncement:announcer:) both the announcement and the announcer >> are passed as argument" >> >> ^ self subscribe: anAnnouncementClass send: aSelector to: anObject >> >> >> could we reduce the cognitive load and make sure that the API is only one >> protocol for example when: send: to:? >> I found it confusing and I have to check that this is indeed the same. >> I think that having one way is far enough. >> >> What do you think? >> Stef >> >> > > > -- > Bernardo E.C. > > Sent from a cheap desktop computer in South America. > -- www.tudorgirba.com "Every thing has its own flow"

Sept. 6, 2015

Re: [Pharo-dev] ***important*** cannot remove a class anymore...

by Peter UhnÃ¡k

> > git clone https://github.com/pharo-project/pharo-core.git > cd pharo-core/ > git show 2536752ecdd88e15b00c6d3e29cb055d108731c8 > But this is post-integration, we want to review things before they break something, not after. So ideally monkey reviews code on CI and if it passes the diff is sent from CI to the mailing list. Also these commits combine several unrelated issues, which is not good for review. Isn't what Squeak uses open-source and available somewhere? Can't we just reuse it? Peter

Sept. 5, 2015

Re: [Pharo-dev] ***important*** cannot remove a class anymore...

by Sven Van Caekenberghe

> On 05 Sep 2015, at 21:34, stepharo <stepharo(a)free.fr> wrote: > > > > Le 5/9/15 19:18, Yuriy Tymchuk a Ã©crit : >> In fact you couldnât spot the issue by just looking at the patch :) there was a very strange thing that could be fixed with one line, but to identify it Iâve spent probably 1.5 hours :) > > sure this is not my point. > my point is that we need to be able to know what changed. Right now I see a title and I do not have the time to click on each github link. > I want to see diffs right in front of my nose. I know very little about git, but the first thing that I tried worked: git clone https://github.com/pharo-project/pharo-core.git cd pharo-core/ git show 2536752ecdd88e15b00c6d3e29cb055d108731c8 I assume that we could put that textual information in the mail. In my terminal, it is coloured, but I still think that the github page is much better: https://github.com/pharo-project/pharo-core/commit/2536752ecdd88e15b00c6d3e… (It happens that this commit changed quite a lot) > Stef >> >> Uko >> >>> On 05 Sep 2015, at 19:03, Nicolai Hess <nicolaihess(a)web.de> wrote: >>> >>> >>> >>> 2015-09-05 11:48 GMT+02:00 Marcus Denker <marcus.denker(a)inria.fr>: >>> >>>> On 05 Sep 2015, at 11:40, stepharo <stepharo(a)free.fr> wrote: >>>> >>>> How can I see the changes? >>>> Our process is not good. Most of us do not get any chance understanding what is changing. >>>> >>> >>> -> download the image before it was added >>> -> merge the slice. >>> >>> Yes, our process is not goodâ¦ but from a review perspective, this issue is the best we can do. >>> *two* reviews, both from people actively contributing to exactly that part of the system. >>> >>> If we require more, we will be back at a process where due to Fear we do nothing. >>> >>> >>> Nautilus is a difficult beast, you all know. Even if you look close at the merge diff you may >>> miss some important relation that only really visible during debugging. >>> I did some test when reviewing this fix. But really, there are so many >>> ways do you need to consider >>> - different ways for opening nautilus >>> - navigate with keys and mouse >>> - selecting updating / changes from other browser and system changes. >>> - refactorings >>> >>> It is really not that easy and I am happy that Franck and Yuri are actually trying to >>> improve Nautilus. >>> you can not easily tests all and make the changes and reviews bullet proof. >>> >>> >>> nicolai >>> >>> >>> >>>> >>>>> Hi, >>>>> >>>>> This is a side effect of >>>>> >>>>> https://pharo.fogbugz.com/f/cases/16475/Nautilus-sends-too-many-announcemen… >>>>> >>>>> (which was reviewed by two people, so not obvious). >>>>> >>>>> What happened is that #updatePackageGroupAndClassList calls itself via #selectedClass: leading to a loop. >>>>> >>>>> Should be easy to fix for the people involved in case 16475. >>>>> >>>>> Marcus >>>>> >>>>> >>>>> On Sat, Sep 5, 2015 at 8:51 AM, stepharo <stepharo(a)free.fr> wrote: >>>>> Hi >>>>> >>>>> I do not know if this is linked to recent changes but we cannot remove classes or move them to another package. >>>>> >>>>> Stef >>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> -- >>>>> Marcus Denker -- denker(a)acm.org >>>>> http://www.marcusdenker.de >> >

Sept. 5, 2015

Re: [Pharo-dev] when:send:to: vs. subscribe:send:to:

by Bernardo Ezequiel Contreras

Excuse me, but what about the following messages MessageBrowser browse: ({WeakSubscriptionBuilder>>#on:do:. WeakSubscriptionBuilder>>#on:send:to:}) should be deprecated, or not? On Sat, Sep 5, 2015 at 5:24 PM, stepharo <stepharo(a)free.fr> wrote: > Hi > > in announcer we have > > when: anAnnouncementClass send: aSelector to: anObject > "Declare that when anAnnouncementClass is raised, anObject should > receive the message aSelector. > When the message expects one argument (eg #fooAnnouncement:) the > announcement is passed as argument. > When the message expects two arguments (eg > #fooAnnouncement:announcer:) both the announcement and the announcer > are passed as argument" > > ^ self subscribe: anAnnouncementClass send: aSelector to: anObject > > > could we reduce the cognitive load and make sure that the API is only one > protocol for example when: send: to:? > I found it confusing and I have to check that this is indeed the same. > I think that having one way is far enough. > > What do you think? > Stef > > -- Bernardo E.C. Sent from a cheap desktop computer in South America.

Sept. 5, 2015

Re: [Pharo-dev] ***important*** cannot remove a class anymore...

by Ferlicot D. Cyril

Le 05/09/2015 21:39, stepharo a Ã©crit : > > > Le 5/9/15 16:49, Yuriy Tymchuk a Ã©crit : >> I can give you a short explanation for the sake of more people knowing >> how Nautilus works. > Yes please if you have some time. > What I do not like is that we pass nil to indicate that there is no > selection. I was thinking that an hierarchy of null objects could help > but this is difficult to > foresee if it will make sense. > I also that all the logic update without reselecting is complex. > I started to read the code and also move some condition about the model > to the model class :). > I would like to do just a simple pass reading all the code and not > changing much to get now yet another better feel of nautilus. > I will also remove the Nautilus help. > I would like to rename the NautilusRefactoring because it is not about > nautilus but about RB. > >> >> Regarding diffs and knowledge sharing, we should have a normal code >> review support with something like gerrit. > > thomas was working on a code review tool and skip is taking over so I > hope that we will have it. >> >> Uko >> > > I am also interesting for the informations about Nautilus :) Each time I try to check some code from it it's Hell! -- Cyril

Sept. 5, 2015