Hi,

There is clearly no downside of having more detailed diff information available. We do not have it now, and the proposal of Stef is more than reasonable.

It would be great to have someone willing to spend a bit of time helping him to achieve this. Who would like to give this a try?

Cheers,
Doru


On Sun, Sep 6, 2015 at 10:28 AM, stepharo <stepharo@free.fr> wrote:


Le 6/9/15 00:33, Sven Van Caekenberghe a ��crit :
On 05 Sep 2015, at 21:34, stepharo <stepharo@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/2536752ecdd88e15b00c6d3e29cb055d108731c8

(It happens that this commit changed quite a lot)

Stef
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

https://pharo.fogbugz.com/f/cases/16475/Nautilus-sends-too-many-announcements-for-a-single-action

(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




--
--
Marcus Denker�� --�� denker@acm.org
http://www.marcusdenker.de







--
www.tudorgirba.com

"Every thing has its own flow"