[PULL REQUEST] GSoC Printing: initial batch of patches

[Lubomir I. Ivanov]

On 4 June 2015 at 20:02, Dirk Hohndel <dirk at 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?

> 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
--