search

RaspberryPiFoundation / lesson_format

Lesson formatter
17 stars 28 forks source link

Don't Overwrite Existing PDFs with Generated Ones #156

Closed martinpeck closed 9 years ago

martinpeck commented 9 years ago

If a project already has PDFs available then we should not overwrite them/replace them with ones generated from the markdown files. We should assume that the PDFs supplied are supplied for good reason.

At present, we will use the existing PDFs when build.py is run without PDF generation, and we will ignore/replace them if run with PDF generation. This behaviour is, at best, confusing. So, we should remove this distinction and simply do the same thing in both cases; use the existing PDFs.

andylolz commented 9 years ago

Looks good to me. Looks like I attempted to do this in 06793788, but got it wrong :\

In practice, I think the only occasions where project PDFs are specified are when there’s no markdown (e.g. ar-EG) in which case the PDF generation won’t run anyway. So I don’t think this changes the output in practice?

We should assume that the PDFs supplied are supplied for good reason

In case it’s useful: when this code was added, PDFs were supplied because there was no automatic PDF generation. So it made sense to ignore the supplied PDFs and just generate anyway. Might help explain other bonkers-looking code!

martinpeck commented 9 years ago

Thanks Andy. I gave myself a headache looking at the double-negative boolean checks that this code was performing :)

--  Martin Peck Sent with Airmail

On 9 April 2015 at 11:38:11, Andy Lulham (notifications@github.com) wrote:

Looks good to me. Looks like I attempted to do this in 0679378, but got it wrong :\

In practice, I think the only occasions where project PDFs are specified are when there’s no markdown (e.g. ar-EG) in which case the PDF generation won’t run anyway. So I don’t think this changes the output in practice?

We should assume that the PDFs supplied are supplied for good reason

In case it’s useful: when this code was added, PDFs were supplied because there was no automatic PDF generation. So it made sense to ignore the supplied PDFs and just generate anyway. Might help explain other bonkers-looking code!

— Reply to this email directly or view it on GitHub.

andylolz commented 9 years ago

Haha yep, me too! Resolving to if not not False is probably A Bad Thing.

andylolz commented 9 years ago

This is presumably closable now?

martinpeck commented 9 years ago

Sure is. Closing it in 5..4..3..2..