Review Request 111042: Upstream SUSE specific changes and introduce a DISTRO switch to enable

[Martin Gräßlin]

> On June 15, 2013, 7:06 p.m., Albert Astals Cid wrote:
> > Is this some kind of joke?
> > 
> > Honestly upstreaming patches is good, but when they make sense, and adding billions of "If #SUSE" to our code does not make any sense. Please discard this review, break stuff in separate patches and if they make sense we can discuss adding them upstream without the "IF SUSE".
> 
> Johannes Obermayr wrote:
>     Do you remember on http://tsdgeos.blogspot.de/2010/05/complex-systems-are-complex.html and how long I had to study openSUSE's kdelibs4 package which of the patches were applied and which not, rebuilding packages, installing -debuginfo, etc.? Last sentence is one of the reasons for my wish to get also distro specific patches upstream - not only openSUSE's. 
>     If I remember right there were and should be some distro specific bug reports on bko: The more people have easy access to source code in "Complex systems [which] are complex" the likelier reasons for specific bugs can be tracked down ...
>     
>     Yes, binding upstream developers to downstream changes/problems is not really nice but could also mean a win-win-situation for both: downstream devs will do more upstream work and upstream devs can review downstream work, better find distro specific bugs and assign them to downstream devs.
>     
>     Intention of this review request is to start a discussion ...

Upstreaming the patches with if #SUSE doesn't solve the problem: one still doesn't know which code is run by a user's setup. Even worse: the chances for bitrot increase. The current system of downstream patches mean that the distribution has to ensure that the patches apply on new versions. A human has to look at each of them. That's the cost of having downstream patches. If you currently not do that (e.g. debug areas for non shipped software) you should rethink whether you have too many patches.

For downstream patches there are the following possible solutions:
* Try to upstream them: if upstream accepts them you get rid of the patch, if upstream doesn't accept you should best drop the patch, too
* For distro-specific behavior: add abstraction with plugin system and upstream
* keep patches for build issues (common example: compile errors with non C++ compliant compilers)

Personally I would go so far that we as upstream say that we don't accept bug reports in case the distributions carries downstream patches.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111042/#review34388
-----------------------------------------------------------


On June 15, 2013, 6:27 p.m., Johannes Obermayr wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111042/
> -----------------------------------------------------------
> 
> (Updated June 15, 2013, 6:27 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> Distributions should upstream their patches / changes:
> - Upstream / other distributions can easily see distro specific changes and enable them by default by removing "#if defined(DISTRO_xxx)"
> - Maybe duplicate work can be avoided and other distributions can easily use them by "|| defined(DISTRO_xxx)"
> - Less adaptions of downstream patches ...
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 705c84e 
>   kdecore/config/kconfig.cpp 048605d 
>   kdecore/config/kconfig_p.h 7751242 
>   kdecore/config/kconfigdata.h e5dd7da 
>   kdecore/config/kconfiggroup.h 8eddfd5 
>   kdecore/config/kconfiggroup.cpp 9e73eb7 
>   kdecore/config/kdesktopfile.h 1c4eae6 
>   kdecore/config/kdesktopfile.cpp 54e5910 
>   kdecore/kdebug.areas 29a4415 
>   kdecore/localization/klocale_kde.cpp b010e74 
>   kdecore/services/kservice.h 3843bad 
>   kdecore/services/kservice.cpp e2cc86f 
>   kdecore/services/kservicegroup.h 9fdf2b0 
>   kdecore/services/kservicegroup.cpp 08bc587 
>   kdecore/services/kservicegroup_p.h 5f21497 
>   kded/vfolder_menu.cpp f0b0b35 
>   kdesu/defaults.h 706a088 
>   kdeui/kernel/kglobalsettings.cpp 2e3a7eb 
>   khtml/html/html_objectimpl.cpp f0f590d 
>   kio/CMakeLists.txt 4854829 
>   kio/kio/kprotocolmanager.cpp 05bb547 
>   kio/kio/krun.cpp ad5656e 
>   kjs/collector.cpp cdd8421 
>   plasma/containment.h e725a99 
>   plasma/containment.cpp fc2ca70 
>   plasma/private/containment_p.h 75a6f80 
>   plasma/theme.cpp 4554de7 
>   suseinstall/CMakeLists.txt PRE-CREATION 
>   suseinstall/kbuildsycocaprogressdialog.h PRE-CREATION 
>   suseinstall/kbuildsycocaprogressdialog.cpp PRE-CREATION 
>   suseinstall/ksuseinstall.h PRE-CREATION 
>   suseinstall/ksuseinstall.cpp PRE-CREATION 
>   suseinstall/ksuseinstall_export.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/111042/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Johannes Obermayr
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130616/4d513ff4/attachment.htm>