[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdelibs/kdeui/shortcuts
From:       Albert Astals Cid <aacid () kde ! org>
Date:       2009-05-16 11:27:34
Message-ID: 200905161327.35023.aacid () kde ! org
[Download RAW message or body]

A Dissabte, 16 de maig de 2009, Roland Harnau va escriure:
> 2009/5/16 Albert Astals Cid <aacid@kde.org>:
> > A Divendres, 15 de maig de 2009, Roland Harnau va escriure:
> >> The only people you can blame  are the developers of Konquest, they
> >> should have tested the accelerators before they shipped the app. The
> >> api doc for KCheckAccelerators makes the the intended use case
> >> abundantely clear:
> >>
> >> "This class allows translators (and application developers) to check
> >> for accelerator conflicts in menu and widgets."
> >>
> >> This class (with its heavy-weight event filter) is clearly intended
> >> for development and  not for  release time. So please revert your
> >> commit.
> >
> > Yeah they should, but KDE has had this behaviour (auto adding
> > accelerators) since KDE 3 times (and that's a lot of time) and i can show
> > you more than 10 programs that rely on this behaviour in 5 minutes. So
> > doing this change so close to the KDE 4.3 release on a code you don't
> > maintain (and Nick does not either) is something you should not have done
> > without contacting k-c-d
>
> The patch was commited at Mon Feb 16, surely not "close to the KDE 4.3
> release", so the authors had enough time to fix their accelerators. It
> is clearly the conceptually right way and Nick was the last one who
> touched the code.

No, they had no time to fix anything since you did not warn anybody.
Nick being the one that touched the code doesn't automatically make him 
someone that knows the code. This is not your fault, he should have said "i'm 
not so good at that code, let's ask someone else to be really sure"

>
> >> >> > Please next time you touch something that critical in kdelibs get
> >> >> > review from someone
> >> >>
> >> >> I mailed Nick Shaforostoff   before I changed the default and he did
> >> >> not object. For an explanation see
> >> >
> >> > No offense to Nick but he mistook here
> >>
> >> Not at all. In any case I would  appreciate if you do not spread wild
> >> speculations about review issues in public commits. You could have
> >> asked first.
> >
> > Ok, seems you are taking it to the personal level, i'm not attacking you
> > at all [...]
>
> Is this a joke? 

No

> You have unilaterally reverted a patch without asking
> me, k-c-d or the reviewboard, together with a vacuous comment (breaks
> KDE behaviour) and an outright lie (the review issue). So practise
> what you preach. 

Yes, because i was bringing things to they place they should be.

Once things are where they should be we can start discussing again if we want 
auto accelerator happening or not.

May this will surprise you, but i'm totally against auto accelator addition, 
but that is not the moment nor the way to "fix" it. Let me warn you that 
dfaure for example is totally in favor of it, so you'll have a hard time 
discussing with him.

My initial suggestion would be this:
 * Add it just after KDE 4.3 is realeased so people have 6 months to fix the 
problems
 * Warn k-c-d and kde-devel about the change
 * When running in debug mode default to accelerator checking but showing the 
developer/bleeding-edge-user a dialog telling "you forgot to set this 
accelerators" and "these accelerators collide" so it's easy to see there are 
problems with accelerators
 * When running in non-debug default to non accelerator checking mode
 
> And for my "track record": My only somewhat
> significant contribution to kdelibs is  r830140,  a patch  commited by
> Thiago nearly a year ago. So I am not so unexperienced regarding
> kdelibs as my  svn history may suggest. And now I'am biting my tongue.

I'm sorry if you are upset.

Albert

>
>
> Roland



[prev in list] [next in list] [prev in thread] [next in thread]