On Thu, Jun 4, 2015 at 8:13 PM, Lubomir I. Ivanov <neolit...@gmail.com> wrote:
> On 4 June 2015 at 20:02, Dirk Hohndel <d...@hohndel.org> 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? > Sure, I ll push a fix for that. > > > 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 > -- > -- regards, Gehad
_______________________________________________ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface