> On Feb. 25, 2013, 1:01 p.m., Arjen Hiemstra wrote: > > CMakeLists.txt, lines 31-43 > > <http://git.reviewboard.kde.org/r/109071/diff/2/?file=115185#file115185line31> > > > > Why not put these definitions into separate files like is already done > > with other macros like calligra_build_test?
Yes, properly better. Moving all productset related macros into a separate file. Perhaps these macros could be even reused by other projects, so trying to make them not too Calligra-specific for now, besides namespacing. > On Feb. 25, 2013, 1:01 p.m., Arjen Hiemstra wrote: > > CMakeLists.txt, line 120 > > <http://git.reviewboard.kde.org/r/109071/diff/2/?file=115185#file115185line120> > > > > Like you have already mentioned, these product sets can be put into > > separate files as well. I would do that in this patch, I see no reason not > > to. > > Okay, done. Also added a small README describing how own productsets can be added (as I implemented initial support for that as well). > On Feb. 25, 2013, 1:01 p.m., Arjen Hiemstra wrote: > > CMakeLists.txt, line 121 > > <http://git.reviewboard.kde.org/r/109071/diff/2/?file=115185#file115185line121> > > > > Please make productset matching case insensitive so we avoid cases > > where things fail due to people writing -DPRODUCTSET=Creative etc. No definite opinion on that myself, so case-insensitiveness added. > On Feb. 25, 2013, 1:01 p.m., Arjen Hiemstra wrote: > > CMakeLists.txt, line 205 > > <http://git.reviewboard.kde.org/r/109071/diff/2/?file=115185#file115185line205> > > > > If you want to support and encourage use of alternate productsets, I > > would not use an empty default but rather print an error when an > > unrecognised product set is used. This helps people developing new product > > sets since they will know when things go wrong. Agreed. Changed the code to just fail if no matching productset is found. > On Feb. 25, 2013, 1:01 p.m., Arjen Hiemstra wrote: > > libs/CMakeLists.txt, line 18 > > <http://git.reviewboard.kde.org/r/109071/diff/2/?file=115191#file115191line18> > > > > Why not add a SHOULD_BUILD_KOPAGEAPP or so for this? Since it is a > > library it is not inconceivable that someone wants to build it without > > building Flow or Stage. That means to completely "producterize" all of Calligra already. Hm. Perhaps you are right and I should see to go all the way in this patch already. So I have to find a way to generalize the inter-product dependencies... - Friedrich W. H. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109071/#review28007 ----------------------------------------------------------- On Feb. 24, 2013, 12:11 a.m., Friedrich W. H. Kossebau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109071/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2013, 12:11 a.m.) > > > Review request for Calligra. > > > Description > ------- > > PRODUCTSET is a substitute for the old non-exclusive options CREATIVEONLY and > TINY (which then are handled exclusively, eek), with migration support for > CREATIVEONLY flag. Predefined hardcoded productsets are ACTIVE, CREATIVE, > DESKTOP, and ALL (as fallback and default). > > Patch also turns buildsystem to have a SHOULD_BUILD for each product > (app/plugin), which then all get turned on centrally in groups depending on > the productset, instead of everywhere having overlapping and hard to oversee > if-else blocks deciding what gets build and what not. > > Not the perfect final solution, but a first step into the right direction > IMHO. > > Known issues: > * BUILD_AUTHOR is not yet set > > Patch can be also tested as branch addProductSetBuildParameter-kossebau. > > > Diffs > ----- > > stage/part/stage.desktop 447858f > words/CMakeLists.txt e6336a2 > words/app/CMakeLists.txt PRE-CREATION > words/app/Info.plist.template PRE-CREATION > words/app/main.cpp PRE-CREATION > words/app/words.desktop PRE-CREATION > words/part/CMakeLists.txt 56b8c6f > words/part/Info.plist.template 97e1728 > words/part/main.cpp 875eb5d > words/part/words.desktop 35bc4c3 > CMakeLists.txt 564c0a0 > devtools/CMakeLists.txt f0e7b62 > extras/CMakeLists.txt 70edca7 > filters/CMakeLists.txt 5acecef > filters/sheets/CMakeLists.txt 351a8e2 > filters/words/CMakeLists.txt 0c2107c > libs/CMakeLists.txt 2036cf5 > plugins/CMakeLists.txt 0e87b1e > sheets/CMakeLists.txt 9f96e41 > stage/CMakeLists.txt 94dd31c > stage/app/CMakeLists.txt PRE-CREATION > stage/app/Info.plist.template PRE-CREATION > stage/app/main.cpp PRE-CREATION > stage/app/stage.desktop PRE-CREATION > stage/part/CMakeLists.txt de57a0f > stage/part/Info.plist.template 857a8d7 > stage/part/main.cpp 5ef9509 > 3rdparty/CMakeLists.txt a300bd2 > > Diff: http://git.reviewboard.kde.org/r/109071/diff/ > > > Testing > ------- > > > Thanks, > > Friedrich W. H. Kossebau > >
_______________________________________________ calligra-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/calligra-devel
