On Apr 13, 2009, at 12:53 PM, Krzysztof Kosiński wrote:

Here is some detailed explanation I couldn't write earlier. I'm not

surprised by Jon's reaction since I nuked a large portion of the file in

question. This wasn't refactoring though. I replaced a lot of rather weird

and undocumented caching code (it was trying to add each icon as a stock

item, which is not the right thing to do when using named icons) with 1 line

adding builtin icons, because I discovered that doing so fixes the icons in

the eraser toolbar: They are no longer blurred. There is a weird drawing

issue instead where part of the icon is repeated, this may a bug in the

drawing function of SPIcon. I also removed a function that was never

actually executed to reduce the amount of code that needs to be further

scrutinized. I tested this in GDB by setting a breakpoint. I found out why

the named icons were not used on Windows: the index.theme file was missing


Here is the big problem


There was code in there for a reason

You did not understand the code

Instead of talking with someone on Jabber you just nuked things you did not understand

Instead of listening to reasons and *testing* things you were asked to just test, you changed code without paying attention.

So you've masked a symptom by layering on workarounds. However you did not actually fix a bug, and introduced several regressions and additional bugs.

Oh, and setting a breakpoint to determine a function is not used DOES NOT WORK. You *MUST* search the codebase for references.


REMEMBER! this is cross platform code that is built against many different versions of many optional libraries. Code inspection is really needed for changes

(BTW, the same problem scenario existed for breaking the whiteboard. You didn't have a test configuration with that code endabled, so you did not notice when you  broke the whiteboard).


*Please* actually inspect the code before nuking it... and definitely talk with others if you see something "undocumented" that you want to change. You really need to understand code before changing it carelessly.