On 4 June 2015 at 20:02, Dirk Hohndel <[email protected]> wrote: > On Thu, Jun 04, 2015 at 05:58:10PM +0300, Lubomir I. Ivanov wrote: >> So this is the initial work from Gehad. I'm looking forward to getting this >> in >> master as he needs to continute the work into the template specific logic. >> Rebasing all the time while waiting on me for reviews must be a pain, so this >> is pending. > > OK, I added a few comments, some are arguably cosmetic, some are about > hardcoding things. None should prevent me from pulling this in order to > make your life easier. > > But please make it a high priority to address the comments I made on > github. >
replies: https://github.com/torvalds/subsurface/commit/3f3937f908b5c0044aef1caa196511469103daa7#commitcomment-11525543 > Why is this hard coded to A4? This should simply use the printer's default > page size intentional. i did not object about these hardcoded values for testing purposes. *any* size should be supported in the final implementation, otherwise it would be incomplete. https://github.com/torvalds/subsurface/commit/75b5d7e9e34a694a47f8ce01cf90bcf31fbc98eb#commitcomment-11525638 > I realize that using NO_PRINTING is consistent with what we do elsewhere. > Still... > if(NOT NO_PRINTING) sounds really silly > let's turn this around into one if/else/endif that starts with > if(NO_PRINTING) > -- handle that case > else() > -- handle the printing case > endif() Gehad, can you fix what Dirk requests here? > I will however turn this code off by default until we figured out the > building of Grantlee > "from source" for all targets might be the correct choice to present in the docs. >> As discussed this does break the current printing module in the expense of >> not maintaining a couple of modules (old vs new) durring GSoC. >> For users that follow master and don't want to install Grantlee please use >> "cmake -DNO_PRINTING" but mind that Grantlee will be a hard dependency if you >> ever want to print with Subsurface. > > Nope, for now we'll do it the other way around. You'll need to add > -DNO_PRINTING=OFF in order to enable this in master > should Gehad modify the cmake file so that NO_PRINTING is ON by default? >> Some of the patches at this point simply remove old code and add some of the >> new logic which is WIP for the time being. >> >> Earlier today I've sent another patch, which is for the sake of me being able >> to build with NO_MARBLE: >> [PATCH] GlobeGPS: add empty function for NO_MARBLE > > I'll add that one as well. > thanks. lubomir -- _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
