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.