[Development] Some QIcon::fromTheme() enhancements

[Albert Astals Cid]

On Friday 23 August 2013 09:48:26 Simon Hausmann wrote:
> 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. 

Honestly, I don't have any stake at the icon fixes, I don't even know if they 
are right, don't assume I'm here because of my @canonical.com address i share 
with Antti.

> 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)

Please let's not resort to "stop attacking people" as a way to stop people 
disagreeing.

I'm just wondering why he writes "only P1 for stable" when it's clear this 
rule is not being applied by just reading the first page of 
https://codereview.qt-project.org/#q,status:open+project:qt/qtbase+branch:stable,n,z

I don't mind if he says "These are new features so they need to go into dev", 
but I'm sorry "this is not a P1 so can't go into stable" does not sound to me 
as a valid reason.

To me something either it is a bug and has to be fixed ASAP or it is a feature 
and can wait. Of course I'm new here so if we have a policy on why we delay 
fixing bugs I'd be happy to be pointed to it.

Cheers,
  Albert

> 
> 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