***important*** cannot remove a class anymore...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
42 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: ***important*** cannot remove a class anymore...

stepharo
What I would like is to have all the changes after one integration with
+ and - showing the changes.

I grouped a bit my analysis in
https://pharo.fogbugz.com/f/cases/16491/Revisiting-TextDiffBuilder
But now I should finish the other tasks I have pending.

I should finish also some issues I started around announcers.

Stef


Le 6/9/15 15:50, Peter Uhnák a écrit :
> But this is all implementation details, I'm still not sure what
> exactly we (Stef) want to send.
>
> * change diff after each successful CI build for an issue?
> * diff after the change has been integrated?
>
> Peter


Reply | Threaded
Open this post in threaded view
|

Re: ***important*** cannot remove a class anymore...

Uko2
In reply to this post by stepharo
Ok, so here it goes.


This method was setting a state of the model by sending #selectedPackage:, #selectedClass:, ……… to self. Each of this 4 methods were announcing if the parameter was not nil. As you may guess #package:class:category:method: was used through the whole system with occasional nil params.

I hate working with nils. So what I did was to break the method into 4:
#package:
#package:class:
#package:class:protocol:
#package:class:protocol:method:

So now there is no nil passing anymore. E.g. if you select only a class, you send #package:class:. Then I’ve moved announcements from #selectedPackage:, #selectedClass:, ……… to the newly created 4 methods. Also now announcements know the parent scope i.e. NautilusProtocolSelected besides protocol knows also about the class and package that played a role on the selection.

This are essentially all the changes. The issue that occurred later (when selecting a package class was not deselected) is because #selectedClass: was doing `self selectedProtocol: nil`, same with protocol. But #selectedPackage: was not “deselecting” a class, probably because it was handled by calling `model package: SomePackage class: nil category: nil method: nil` in a first place.




On 05 Sep 2015, at 21:39, stepharo <[hidden email]> wrote:



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 <[hidden email]> 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 <[hidden email]> 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









123