- Pharo-dev - lists.pharo.org

Re: [Pharo-dev] how do I push fixed upstream?

by Andrei Chis

To push it you need to make a version for that project. Then if that version only fixes a single bug you can put the issue on "fix review needed" and fill in the fields ConfigurationOf and ConfigurationVersion from the issue. Alternatively, if you need to integrate more issues, you can put those issues on fixed upstream and create a new issue that you put on "fix review needed", fill in ConfigurationOf and ConfigurationVersion and add in the description the issues that it addresses. For gtools we usually uses the second approach and integrate when there are 2-3 issues on fixed upstream. I can make a new version for glamour to integrate on monday (I'm travelling tomorrow) but if you want you can give it a try. Creating a new version for glamour with versioner should be easy :) Cheers, Andrei On Sat, Sep 5, 2015 at 8:51 PM, Peter UhnÃ¡k <i.uhnak(a)gmail.com> wrote: > How do I push "fixed upstream"? > > Since it doesn't have "Fix Review Needed" nobody will see it, review it > and merge it. How should I mark it? Do I need to make another step? > > > https://pharo.fogbugz.com/f/cases/15920/Cannot-inspect-a-SpecTreeNodeModel-… > > Peter >

Sept. 5, 2015

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

by stepharo

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 > >> On 05 Sep 2015, at 15:46, stepharo <stepharo(a)free.fr> wrote: >> >> As you wish. Thanks for your time. >> >> Now I will get really frustrated because I will not know simply what you did and I will have missed a good occasion >> to learn something. This is what sending the diff in text simply give us. >> >> Stef >> >> >> Le 5/9/15 14:09, Yuriy Tymchuk a Ã©crit : >>> I found the issue. Fix almost ready. Should I reopen the case, or create a new one? >>> >>> Uko >>> >>> >>> >>>> On 05 Sep 2015, at 13:59, Stephan Eggermont <stephan(a)stack.nl> wrote: >>>> >>>> On 05-09-15 08:51, stepharo wrote: >>>>> Hi >>>>> >>>>> I do not know if this is linked to recent changes but we cannot remove >>>>> classes or move them to another package >>>> I can move them to another package using drag-and-drop (issue 16488) >>>> in 50305 >>>> >>>> >>>> Stephan >>>> >>>> >>> >> > >

Sept. 5, 2015

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

by stepharo

Le 5/9/15 20:46, Peter UhnÃ¡k a Ã©crit : > Just because looking at a patch isn't as powerful doesn't make it > useless. There are still benefits > > * you are more aware of code changes > * you can still spot conceptual problems > * it's dead simple; the mail literally arrives at your doorsteps and > all you need to do is look > > of course this isn't replacing proper review, but there is big still > value in it. 100% what I want to say. I will allocate time on a goodies that take a slice and produce a diff > > But even the "regular" process could be improved. > >> -> download the image before it was added >> -> merge the slice. >> > For example I have startup script, that for images that have name > "issuenumber - verify" on startup automatically opens MCBrowser on the > slice so I can merge with two clicks. > (It's not automerged because I want to test behavior before and after > loading the slice.) > > Since I'm mostly verifying my own fixes, I generally don't look at the > changesets, but maybe you could autoopen a diff or something... > Or even add a category to PharoLauncher "Pharo - To Review" so I > wouldn't even need to type in the issue number. > > Peter

Sept. 5, 2015

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

by stepharo

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. Stef > > Uko > >> On 05 Sep 2015, at 19:03, Nicolai Hess <nicolaihess(a)web.de >> <mailto:nicolaihess@web.de>> wrote: >> >> >> >> 2015-09-05 11:48 GMT+02:00 Marcus Denker<marcus.denker(a)inria.fr >> <mailto:marcus.denker@inria.fr>>: >> >> >>> On 05 Sep 2015, at 11:40, stepharo <stepharo(a)free.fr >>> <mailto:stepharo@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 >>>> <mailto:stepharo@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 <mailto:denker@acm.org> >>>> http://www.marcusdenker.de <http://www.marcusdenker.de/> >> >

Sept. 5, 2015

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

by stepharo

Oh yes I know. I'm not saying that looking at the changes we would have spotted the problems. I'm just saying that I want to get the changes in textual format in my mail-box as in squeak because it helps me understanding and knowing that something changed. I'm digging in the Diffbuilder and friends to see but I have some other tasks to finish first. > 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 >>> <mailto:stepharo@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 <mailto:denker@acm.org> >>> http://www.marcusdenker.de <http://www.marcusdenker.de/> >> > >

Sept. 5, 2015

Re: [Pharo-dev] rb highlighting is not up to the task yet

by Tudor Girba

Great. I also think we should push in this direction. So, I will continue testing and provide feedback. Please ping me when you modify something. Ok? Cheers, Doru On Sat, Sep 5, 2015 at 9:15 PM, Nicolai Hess <nicolaihess(a)web.de> wrote: > I do, working with RBStyler makes much more fun than with the other one > and working > with AST instead of plain code text, gives us many possibilities to create > great tools (highlighter for example:) ) > > -> > pharo.fogbugz.com/f/cases/16492/no-syntax-highlighting-if-last-expression-i… > > I think we should not go back to the old one, but force the work on the rb > styler. > Just report any issues you see. > > > 2015-09-05 20:29 GMT+02:00 Tudor Girba <tudor(a)tudorgirba.com>: > >> Hi, >> >> Yes, indeed. I think it is already much better than it was some months >> ago, and I definitely think it is a great goal to have everything around RB. >> >> My question was only if there is anyone that invests in this actively >> because perhaps we can work together? >> >> Cheers, >> Doru >> >> >> >> On Sat, Sep 5, 2015 at 6:34 PM, Marcus Denker <marcus.denker(a)inria.fr> >> wrote: >> >>> Hello, >>> >>> Yes, I am aware of that. >>> >>> The idea was to evaluate how good the RB AST colouring is already and >>> maybe find the energy to >>> fix all remaining problemsâ¦ we fixed many but not all. >>> >>> I propose that we keep it for some month more and if we donât improve >>> these problems go >>> back to the old scheme for Pharo5. >>> >>> Then we can re-start to work on this in Pharo6. >>> >>> >>> Marcus >>> >>> On 05 Sep 2015, at 07:28, Tudor Girba <tudor(a)tudorgirba.com> wrote: >>> >>> Hi, >>> >>> Here is an example. >>> >>> Type this in a playground in a Pharo 5 image, and all statements before >>> the parentheses become black: >>> | a b c d | >>> a := 42. >>> b := a + 1. >>> c := b + 1. >>> d := c + 1. >>> a . b . c . d } >>> >>> <Playground-50.png> >>> >>> Here is how this looks in a Pharo 4: >>> >>> <Playground-40.png> >>> >>> Cheers, >>> Doru >>> >>> >>> On Sat, Sep 5, 2015 at 12:16 AM, Nicolai Hess <nicolaihess(a)web.de> >>> wrote: >>> >>>> This depends on the kind of error. For example, a missing open or >>>> closed parenthesis will most >>>> likely mark the whole (following) code. >>>> >>>> But it should still highlight the preceding code. >>>> Can you give an example. >>>> >>>> >>>> nicolai >>>> >>>> >>>> 2015-09-04 23:10 GMT+02:00 Tudor Girba <tudor(a)tudorgirba.com>: >>>> >>>>> Hi, >>>>> >>>>> I think it's nice that we are going towards consolidating our tools >>>>> around RB. >>>>> >>>>> However, the highlighting logic is not quite enough because if I have >>>>> an error anywhere in the code, the whole highlighting disappears. In >>>>> contrast, Shout manages to limit the problem only to the surrounding code. >>>>> >>>>> Is anyone working on this part of the highlighting? >>>>> >>>>> Cheers, >>>>> Doru >>>>> >>>>> -- >>>>> www.tudorgirba.com >>>>> >>>>> "Every thing has its own flow" >>>>> >>>> >>>> >>> >>> >>> -- >>> www.tudorgirba.com >>> >>> "Every thing has its own flow" >>> >>> >>> >> >> >> -- >> www.tudorgirba.com >> >> "Every thing has its own flow" >> > > -- www.tudorgirba.com "Every thing has its own flow"

Sept. 5, 2015

Re: [Pharo-dev] rb highlighting is not up to the task yet

by Nicolai Hess

I do, working with RBStyler makes much more fun than with the other one and working with AST instead of plain code text, gives us many possibilities to create great tools (highlighter for example:) ) -> pharo.fogbugz.com/f/cases/16492/no-syntax-highlighting-if-last-expression-i… I think we should not go back to the old one, but force the work on the rb styler. Just report any issues you see. 2015-09-05 20:29 GMT+02:00 Tudor Girba <tudor(a)tudorgirba.com>: > Hi, > > Yes, indeed. I think it is already much better than it was some months > ago, and I definitely think it is a great goal to have everything around RB. > > My question was only if there is anyone that invests in this actively > because perhaps we can work together? > > Cheers, > Doru > > > > On Sat, Sep 5, 2015 at 6:34 PM, Marcus Denker <marcus.denker(a)inria.fr> > wrote: > >> Hello, >> >> Yes, I am aware of that. >> >> The idea was to evaluate how good the RB AST colouring is already and >> maybe find the energy to >> fix all remaining problemsâ¦ we fixed many but not all. >> >> I propose that we keep it for some month more and if we donât improve >> these problems go >> back to the old scheme for Pharo5. >> >> Then we can re-start to work on this in Pharo6. >> >> >> Marcus >> >> On 05 Sep 2015, at 07:28, Tudor Girba <tudor(a)tudorgirba.com> wrote: >> >> Hi, >> >> Here is an example. >> >> Type this in a playground in a Pharo 5 image, and all statements before >> the parentheses become black: >> | a b c d | >> a := 42. >> b := a + 1. >> c := b + 1. >> d := c + 1. >> a . b . c . d } >> >> <Playground-50.png> >> >> Here is how this looks in a Pharo 4: >> >> <Playground-40.png> >> >> Cheers, >> Doru >> >> >> On Sat, Sep 5, 2015 at 12:16 AM, Nicolai Hess <nicolaihess(a)web.de> wrote: >> >>> This depends on the kind of error. For example, a missing open or closed >>> parenthesis will most >>> likely mark the whole (following) code. >>> >>> But it should still highlight the preceding code. >>> Can you give an example. >>> >>> >>> nicolai >>> >>> >>> 2015-09-04 23:10 GMT+02:00 Tudor Girba <tudor(a)tudorgirba.com>: >>> >>>> Hi, >>>> >>>> I think it's nice that we are going towards consolidating our tools >>>> around RB. >>>> >>>> However, the highlighting logic is not quite enough because if I have >>>> an error anywhere in the code, the whole highlighting disappears. In >>>> contrast, Shout manages to limit the problem only to the surrounding code. >>>> >>>> Is anyone working on this part of the highlighting? >>>> >>>> Cheers, >>>> Doru >>>> >>>> -- >>>> www.tudorgirba.com >>>> >>>> "Every thing has its own flow" >>>> >>> >>> >> >> >> -- >> www.tudorgirba.com >> >> "Every thing has its own flow" >> >> >> > > > -- > www.tudorgirba.com > > "Every thing has its own flow" >

Sept. 5, 2015

Re: [Pharo-dev] rb highlighting is not up to the task yet

by stepharo

+ 1 We cannot be improving all fronts in parallel. Stef Le 5/9/15 18:34, Marcus Denker a Ã©crit : > Hello, > > Yes, I am aware of that. > > The idea was to evaluate how good the RB AST colouring is already and > maybe find the energy to > fix all remaining problemsâ¦ we fixed many but not all. > > I propose that we keep it for some month more and if we donât improve > these problems go > back to the old scheme for Pharo5. > > Then we can re-start to work on this in Pharo6. > > > Marcus > >> On 05 Sep 2015, at 07:28, Tudor Girba <tudor(a)tudorgirba.com >> <mailto:tudor@tudorgirba.com>> wrote: >> >> Hi, >> >> Here is an example. >> >> Type this in a playground in a Pharo 5 image, and all statements >> before the parentheses become black: >> | a b c d | >> a := 42. >> b := a + 1. >> c := b + 1. >> d := c + 1. >> a . b . c . d } >> >> <Playground-50.png> >> >> Here is how this looks in a Pharo 4: >> >> <Playground-40.png> >> >> Cheers, >> Doru >> >> >> On Sat, Sep 5, 2015 at 12:16 AM, Nicolai Hess <nicolaihess(a)web.de >> <mailto:nicolaihess@web.de>> wrote: >> >> This depends on the kind of error. For example, a missing open or >> closed parenthesis will most >> likely mark the whole (following) code. >> >> But it should still highlight the preceding code. >> Can you give an example. >> >> >> nicolai >> >> >> 2015-09-04 23:10 GMT+02:00 Tudor Girba <tudor(a)tudorgirba.com >> <mailto:tudor@tudorgirba.com>>: >> >> Hi, >> >> I think it's nice that we are going towards consolidating our >> tools around RB. >> >> However, the highlighting logic is not quite enough because >> if I have an error anywhere in the code, the whole >> highlighting disappears. In contrast, Shout manages to limit >> the problem only to the surrounding code. >> >> Is anyone working on this part of the highlighting? >> >> Cheers, >> Doru >> >> -- >> www.tudorgirba.com <http://www.tudorgirba.com/> >> >> "Every thing has its own flow" >> >> >> >> >> >> -- >> www.tudorgirba.com <http://www.tudorgirba.com/> >> >> "Every thing has its own flow" >

Sept. 5, 2015

how do I push fixed upstream?

by Peter UhnÃ¡k

How do I push "fixed upstream"? Since it doesn't have "Fix Review Needed" nobody will see it, review it and merge it. How should I mark it? Do I need to make another step? https://pharo.fogbugz.com/f/cases/15920/Cannot-inspect-a-SpecTreeNodeModel-… Peter

Sept. 5, 2015

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

by Peter UhnÃ¡k

Just because looking at a patch isn't as powerful doesn't make it useless. There are still benefits * you are more aware of code changes * you can still spot conceptual problems * it's dead simple; the mail literally arrives at your doorsteps and all you need to do is look of course this isn't replacing proper review, but there is big still value in it. But even the "regular" process could be improved. > -> download the image before it was added >> -> merge the slice. >> > For example I have startup script, that for images that have name "issuenumber - verify" on startup automatically opens MCBrowser on the slice so I can merge with two clicks. (It's not automerged because I want to test behavior before and after loading the slice.) Since I'm mostly verifying my own fixes, I generally don't look at the changesets, but maybe you could autoopen a diff or something... Or even add a category to PharoLauncher "Pharo - To Review" so I wouldn't even need to type in the issue number. Peter

Sept. 5, 2015