[Development] Some QIcon::fromTheme() enhancements

[Simon Hausmann]

On Friday 23. August 2013 09.24.54 Albert Astals Cid wrote:
> On Friday 23 August 2013 07:50:56 Olivier Goffart wrote:
> > On Thursday 22 August 2013 22:18:54 Antti Kaijanmäki wrote:
> > > On 22.08.2013 21:51, Thiago Macieira wrote:
> > > > On quinta-feira, 22 de agosto de 2013 21:26:15, Antti Kaijanmäki 
wrote:
> > > >> I have patches (linked in the bug) to amend this, and I would like to
> > > >> get some feedback on them.
> > > > 
> > > > Hello Antti
> > > > 
> > > > If you can, please upload the patches to codereview.qt-project.org.
> > > > It's
> > > > easier to discuss them there.
> > > 
> > > Either I was not clear enough or I have misunderstood something, but the
> > > links to the patches in codereview.qt-project.org are in that bug
> > > report.
> > > 
> > > :)
> > 
> > Thanks for the patch, I will have a look.
> > However, the patches do not fix a regression, neither a P1 bug, so they
> > should go to the 'dev' branch instead of the 'stable' branch.
> 
> This is weird
> 
> https://codereview.qt-project.org/#change,63593
> approved by you is linked to a P3
> 
> https://codereview.qt-project.org/#change,63496
> is also linked to a P3
> 
> https://codereview.qt-project.org/#change,63535
> is also linked to a P3
> 
> https://codereview.qt-project.org/#change,63413
> is linked to a P2
> 
> https://codereview.qt-project.org/#change,63595
> is not linked to any bug
> 
> https://codereview.qt-project.org/#change,62408
> is also not linked to any bug
> 
> https://codereview.qt-project.org/#change,62948
> is linked to a bug without categorization

Seriously? Three of those are documentation fixes (so P1 or not doesn't make 
sense). Incorrect device pixel ratio can cause severe misrendering on retina 
displays, so I'd say it's pretty easy to argue that it should be fixed in patch 
release. The second last is a build issue that Olivier had nothing to do with 
and that in Joerg's opinion was important enough to fix in the next patch 
release. The QString fix you could argue fixes data corruption. And I think 
similarly it's easy to talk about the pop-up location fix.

Instead of trying to attack other changes or Olivier's judgement as approver, 
I think it would be much better to explain why it is important that the icon 
fixes should go into a patch release. Make a good case for the patch, respond 
with arguments. And if you don't succeed in convincing Olivier, then there are 
other approvers that might be willing to review this. (I can think of one off 
the top of my head)

FWIW I personally think the icon theme fixes are worth it, but I'm not the one 
who should be making that case.
 

Simon

> Also i don't understand that "P1 rule for stable", what's the rationale to
> not fixing bugs in stable? That's what stable branch is for, no?
> 
> I could understand "P1 rule for release", but for stable, what if i fix a
> small bug (which I understand won't be categorized as P1), why should people
> wait for the fix of that small bug for Qt 5.2 instead of 5.1.x?
> 
> Cheers,
>   Albert
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development