[Development] Some QIcon::fromTheme() enhancements

[Olivier Goffart]

On Friday 23 August 2013 14:23:23 Antti Kaijanmäki wrote:
> On 23.08.2013 11:32, Olivier Goffart wrote:
> > Stable means it does not change much.
> 
> I was very careful not to introduce any big changes. I'm only a default
> path and providing a fallback in the case that the normal loading from
> theme directories fails. I'm not touching the way theme directories are
> travelled or how the engine handles the search or matching of the found
> icons or the icon cache.

First, I want to say that the patches are of great quality.  They have good 
tests. They indeed seem to fix what they claim to fix in a clean way, consistent 
with the rest of the code. Overall, they will be a great addition to Qt once 
they will be in. Thank you for that.

But they are not fitting the criteria for the stable branch.

The bug has been there unreported since Qt 4.7 three years ago.
Being able to open xpm theme can even be seen as a feature.

Who are the affected users? Why is this feature so urgent?

The examples of problems I gave may seem not so plausible, but that's not the 
point.  The point is that we don't know what are the consequences.

If we make an exception for this commit, then why not also for all the 
hundreds of other commits that fixes bug "that do not introduces big changes"


[...]
> My only argument would be that QIcon::fromTheme() does not do what it's
> documented to do. [...]

And that is a weak argument.  More often, the documentation is changed to 
match the behaviour when there is contradiction.

We try to avoid changing behaviour, unless there is a very good reason to do 
so.  We need to be very careful when changing defaults.

Keeping backward compatibility is hard.  Often we have to make tradoffs and 
choices in the behaviour we don't like.  Sad reality :-/ 

If we want to have a stable library, we need to be conservative.

> I don't have any strong feelings about getting this to stable. dev is
> also fine, but I don't see a reason why this could not be included in
> stable. 

I hope I can make you understand better.

> The chances of introducing regression is IMO very low.

That's what everybody says :-)

-- 
Olivier

Woboq - Qt services and support - http://woboq.com - http://code.woboq.org