Wednesday, October 27, 2010

Qt, gitorious, merge requests and Open Source

Begin of rant

Some time ago Qt/Nokia decided to open their repos (great! even if they are not the real repos) and also decided to start accepting contributions from non Qt/Nokia employees in a way that they will make you the favour of accepting your code for free without making you fax things and talk with lawyers (you still have to grant them a perpetual license to do whatever they want with your code).

The fact that you have to grant them a perpetual license to do whatever they want with your code was the reason i decided myself not to contribute to Qt.

But the flesh is weak and i actually found something that i though would be nice to have and was actually easy to implement. So i went ahead and did it (http://qt.gitorious.org/qt/qt/merge_requests/814) seven weeks ago.

Since then my merge request has received exactly two comments. One from a non Qt/Nokia employee saying "Looks good to me (and useful!)" and one from a Qt/Nokia employee saying "Your indentation is wrong" (which i think is not but that's unrelated).

And that's it.

I know phones don't have a need for QHeaderViews, but Qt/Nokia could pretend they still are supporting the desktop (which is what brought them to the position they are) and if they get a patch from a reasonably known community member that gets acked by a well known community member have the decency of following up the patch.

End of rant.

12 comments:

Anonymous said...

I have tried several times to submit patches this way. I have come to the conclusion that getting any patch into QT no matter how trivial is next toimpossible.

kdepepo said...

While I don't like the current way of contribution either, you should slow down a bit.

What your patch requests is not a bug fix, but a feature addition, so it will certainly not be part of the current Qt 4.7 series. Qt 4.8 has not branched yet, and when it does, I am sure they will take some time to evaluate contributions.

And KDE isn't any better. I have recently applied (trivial) patches to bug fixes that have been attached to bug reports over a year ago.

Anonymous said...

Time ago I submitted a short, 15 line patch to optimize one function of one base class. I had comments about style-indentation too. But, specially, they request me a unit test and prove of performance. I wrote the unit test, even though the patch was *not* introducing new functionality. I set too several tests for performance, for several compilers, that were showing a 5 to 10 fold speed increase in that function, in that core class.

I felt abused. The guy was keeping asking, asking, asking, me, more and more things. Their tone was not even polite. At times I had the feeling that him does not know how optimized code looks like. At times the feeling was like me competing with him. He was not helpful. In the end I felt miserable and delete the submission.

Robin Burchell said...

Hi there!

I'm the guy that replied to your MR initially.

I can understand your frustration, it really does look depressing at times that things stay in the queue so long.. all I can say is that I don't think anyone genuinely *wants* this problem to stay as it is.

I review merge requests (at least, as much as I can, I tend to leave the ones I don't know well enough) to try ease the integration process in the longer run, I also periodically do things like running tests to make sure older MRs still merge cleanly, etc. It's my part of trying to oil the wheels a bit, and I sincerely wish I could do more to ease the pain, but... :)

In the long run, please do stick in there. I've bumped this to the Qt guy who reviewed this also, so hopefully he'll find some time to reply again in the future - though he is busy with other tasks.

I'll also reply to a few other comments on here separately, simply because I like to think that the time I've spent doing reviews/etc there have given me some insights.

Robin Burchell said...

@anonymous (number one):
it's not impossible, but it does certainly take effort sometimes. I've gotten around 15-20 patches submitted and merged now, with another few still waiting on review+integration (such as http://qt.gitorious.org/qt/qt/merge_requests/2373 - which will probably wait on 4.8 branching)

Robin Burchell said...

@anonymous (number two):
it may feel like a big task, and a lot of inconvenience at times, yes.

however, keep in perspective that Qt is used in thousands of deployments on .. well, lots and lots of applications, by millions of people. as a side-effect of this, generally speaking, the quality standards are *strict*.

for pretty much any change, you need to make a best effort to prove that you haven't broken functionality in any way (hence the unit tests). unfortunately, some of the older pieces of Qt aren't very well unit tested (because at the time, presumably, there was no such requirement? I don't know). I myself was responsible for writing a lot of unit tests for things like QList/QVector during 4.7's cycle.

anyway, back to the point: this may mean that, at times, you are requested to write tests covering functionality you touched in some way, even if it wasn't adding functionality. this is a burden, but at the same time, an extra hurdle here prevents a lot of angry developers (because their application is broken) and even more angry users. as a platform developer, I'd argue that you have to take responsibility for your users very seriously.

i'd also encourage you to come and talk to people in real time on IRC (#qt-labs), you'll quickly find that they aren't as imposing as you might think :)

Yun-Ta Tsai said...

Hi,

I am not from the Troll team but work in Nokia. Things are that they have to review patches both internally and externally. As I have governed software planning as well, I do understand why they would ask many questions.

They do want to know every detail of the patch. Imagine that you have your project setup on Git, and accept hundreds or thousands of patches, plus countless feature / bug reports in JIRA, etc. Undoubtedly, it takes 'time' to process. More or less, you may want to question these patches too, for security / maintaining purposes.

Please don't feel bad if you get questioned. They question the same way to you as to me. But Qt has to support millions of platform, performance changes from one platform may not benefit to others, especially on embedded system is a totally different story.

I will reflect this comment back to internal and hope they could improve the process. However, Trolls work very hard with open community. I have used and developed Qt/KDE back to a decade ago. There are lots of changes, new people, new software, new features, etc. But one thing hasn't changed is both Qt and KDE are benefit from both side of innovation.

Senior Researcher
System Architecture and Design
Yun-Ta Tsai

Anonymous said...

You could also try making a bug report for this. That way a person gets assigned the bug report and it gets a priority etc.

Anonymous said...

Also your patch doesn't have any unit test, so whoever commits this has to write a unit test for it.


I have submitted a few merge requests complete with unit tests and a Qt bug number that it's fixing, and they get merged fairly fast.

Albert Astals Cid said...

@kdepepo: If the reason they are ignoring merge requests is that 4.8 is not open yet, writing so in the merge request will make everyone happier. Also i doubt you can say KDE is not any better. In Qt i'm on a dead end, there's nothing more i can do to get the patch in, in KDE i can get more and more involved and eventually become the maintainer if i really want my patch commited. And even if KDE was not any better, when is that an excuse for anything?

@Robin: Mostly agree with you

@Yun: I'm not complaining about they asking things, i'm complaining about they ignoring things

@anon: I'm not doing a unit test until they ask for it

John said...

Yeah, I know what you mean, I'm frustrated with the simple lack of feedback too, and the lack of documentation on how the process works. They really need someone allocated to actively managing the merge requests on a regular basis and providing proper feedback, not just whoever has some spare time.

While I got two very small changes in, my larger patch to fix printing has been sat in limbo for ages. While other people made positive noises and talked about doing api reviews, I never heard from the printing guys again. Then the 4.7 feature set was frozen so I shelved everything. Now that 4.7 is out and the 4.8 feature set is being decided, I have to decide if it's worth the effort trying to fight for the patches to be go in and be done right, or just to go ahead and fork QPrinter, get rid of the cruft, and do it right. But that's something to consider after the KDE 4.6 feature freeze.

Anonymous said...

thanks for share.