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 :)

Uko

On 05 Sep 2015, at 19:03, Nicolai Hess <nicolaihess@web.de> wrote:



2015-09-05 11:48 GMT+02:00 Marcus Denker <marcus.denker@inria.fr>:

On 05 Sep 2015, at 11:40, stepharo <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


(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@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




--