LRN-34037 Option 2: Containerise with extra makefile - Githubissues

Learnosity / learnosity-sdk-python

Learnosity SDK for Python

Apache License 2.0

7 stars 10 forks source link

LRN-34037 Option 2: Containerise with extra makefile #58

Closed david-scarratt-lrn closed 2 years ago

david-scarratt-lrn commented 2 years ago

Summary

Add the ability to build and test in a container. The container command is in an external makefile, requiring selection using -f docker.mk.

Pros

Doesn't change the existing Makefile, which is exactly the same whether running locally or in a container
Added file is also very simple
Doesn't require extra control variables (LRN_SDK_NO_DOCKER)
Allows making targets that don't require Python (notably clean) without a container
Safe with Docker in Docker

Cons

Easy to forget to add -f docker.mk—the choice has to be expressed every time make is invoked
Opt-in rather than opt-out
Extra file

Checklist

[x] Feature
[ ] Bug
[ ] Security
[ ] Documentation
[ ] ChangeLog.md updated
[ ] Tests added
[ ] All testsuites passed
[ ] make dist completed successfully

david-scarratt-lrn commented 2 years ago

After discussion, the preference overall is for the other option, so closing this.