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/2536752ecdd88e15b00c6d3e2...
(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-announcement...
(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"