Hi Jimmy (and other IVI folks),
It's great to see you paid attention to my patches
(http://lists.meego.com/pipermail/meego-dev/2010-August/004470.html for
example),
however, I have a few notes.
0) I'd like to be credited for work I do - it's fairly common
practice, and it generally helps encourage people to do more of it in
the future.
This refers to ivihome commits a2f219cee109f2247aacd669c7243be4331fec68,
c5c9eceda864a400418d71849cb01f07f3dd34e3, and commit
d5c8e99be0d301c820e90bed7ba77ea2a101c45a from ividesktop.
I understand that at least the ivihome project commit was modified from my
submission, however, a "based on work by ____" isn't out of place there.
1) my patch notes that matchbox is also a runtime dependency, not just a
build-time dependency. Is there any reason not to include this?
2) there's a typo in the added 'running' section of README ('included' vs
'includes')
3) The project file now sets:
INCLUDEPATH += $$PREFIX/include/libmb/
.. and while this might sound like a good idea, it totally and utterly
defeats the purpose of adding $$PREFIX in the first place, as now, if someone
tries to use qmake PREFIX=/usr/local/, INCLUDEPATH will be incorrectly set,
so it won't build at all. If it did, the install would work correctly,
though. :)
If libmb provides a pkg-config file (which I'm fairly sure it will), then the
proper solution is to set CONFIG += link_pkgconfig, and PKGCONFIG += libmb
(or similar).
4) Not strictly a technical issue, but I personally disagree with shoving
multiple, unrelated changes into a single commit. It tends to make
collaboration and debugging very difficult.
Thanks for integrating!
--
Robin Burchell
http://rburchell.com
_______________________________________________
MeeGo-dev mailing list
[email protected]
http://lists.meego.com/listinfo/meego-dev
