search

RaspberryPiFoundation / lesson_format

Lesson formatter
17 stars 28 forks source link

Remove pandoc_scratchblocks submodule #182

Closed andylolz closed 8 years ago

andylolz commented 8 years ago

This reverts #96.

martinpeck commented 8 years ago

Sorry - I've not been paying attention to this.

Why have we removed the submodule? What does this fix? How should Heroku now build these projects?

andylolz commented 8 years ago

Why have we removed the submodule?

Submodules are tricky to work with. This tool works just as well (if not better) without any. It previously worked without a submodule (see #96).

What does this fix?

This is part of the fix for CodeClub/scratch-curriculum#400. The other option was to update the submodule, which I gave as an alternative in this comment.

More generally, this prevents the possibility of accidentally reverting the submodule.

How should Heroku now build these projects?

I don’t believe any changes are necessary. If heroku still runs make clone then I think this should just work as is.

martinpeck commented 8 years ago

Thanks for this @andylolz, but I've reverted @rikcross's merge of this PR for a couple of reasons...

  1. Heroku gets really upset if you remove a submodule. I believe you have to reset the repository on Heroku to get around this problem. When we tried to push the merged version (without submodules) Heroku failed on push because it was trying to update submodules that didn't exist. I couldn't find anything definitive that confirmed that nuking things would work (or, what other problems this might cause) so I was reluctant to do this.
  2. Better the devil you know. Submodules are a pain at times, but it's a pain we understand.

Anyway, that's the reason why we're living with submodules.

andylolz commented 8 years ago

Righto. I can totally understand not merging this one (or in this case, reverting it). I was really only sending it as an option, and did think it might be a controversial one. With hindsight, I should have written something to this effect in the pull request.

In place of this, the submodule version will need updating. I’ll send a PR for that now.