Enhancement proposal: a simple CI/CD system with GitHub Actions

[ejgenc]

Hello!

We had previously discussed that a simple CI/CD system implemented with GitHub actions could add a lot of value to the project. I feel like it is time to build out the CI/CD system as we add more features around the project. Below is a list of some potential GitHub actions "workflows" we can implement:

• [x] Do code style checks on each pull request to main. Right now, this only means running flake8. If flake8 exits with a negative result, the pull request is flagged/denied.
• [x] Running our test suite to see if tests pass on each pull request to main. If the tests don't pass, the pull request is flagged/denied.
• [ ] Calculating the would-be code coverage ratio on each pull request to main. If the tests don't pass, the pull request is flagged/denied.
• [ ] Running an expanded test suite again on each pull request from master to stable.
• [ ] Automatically building the package & publishing to PyPI on each pull request to stable.
• [x] Automatically building & publishing documentation to Read the Docs on each pull request to stable.
• [ ] Listening in on the original API repositories and running tests on major version releases.

We can use this enhancement proposal as a place to discuss anything related to a potential CI/CD system.

[ejgenc]

18 is the realization of the first item in the checklist above.

[ejgenc]

@joseed-europeana It turns out that I was wrong and black can be integrated as a GitHub action (see here). In theory, the style-check.yaml workflow can be easily extended to include black. However, I believe that we should decide against doing this for the reasons outlined below:

• Making sure that the code complies to a certain style guide should be the concern of the contributor and not of the CI/CD system. The CI/CD system exists as a gatekeeper/security measure & as a way to automate certain tedious actions. Think of this analogy: we run tests through GitHub actions to make sure that nothing breaks. If tests pass, the gate to the repo is opened. If tests do not pass, we do not ask the CI/CD system to go ahead and fix whatever is preventing them from being opened.
• The cost of using pre-commit locally is minimal: it is just one extra command (poetry run pre-commit install) after the initial installation command, which every contributor must do anyhow. The directions are clear on this and the documentation, I believe, is accessible.

This is my opinion. However, I also know that at the scale of our project both options are fine. What do you think?

[joseed-europeana]

I would be fine with using a pre-commit for formatting, no need to include it in the CI/CD system for now

[ejgenc]

23 is the realization of the second item in the checklist. I've set up a GitHub Actions workflow that runs poetry run pytest whenever a pull request concerning the src/ folder or the tests/ folder is made. We currently have no tests in the test suite, but it will work as the test suite expands.

NOTE: the five tests that existed were surpressed because they tested a behavior different than what they were advertising.

[ejgenc]

Upon discussion, we realised that item number 6 (auto building and publishing of Sphinx documentation to Read the Docs) was unnecessary as Read the Docs offered that functionality out of the box.

[ejgenc]

@joseed-europeana Even though we are using the pipeline of Read the Docs to build documentation, we still need to find a way to make sure that the files under docs/source actually build before updating the stable branch w/ the master branch. Right now, Sphinx + RTD is failing silently without any checks. Do you think we should write a GitHub action that builds Sphinx documents in a VM so that we can check that the documentation builds properly?

[joseed-europeana]

Well spotted, this would be very convenient. The current state is that RTD will get updated whenever the stable branch does, but indeed we don't check whether the documentation builds correctly when the stable branch is updated. Feel free to create an action

[ejgenc]

@joseed-europeana We now have the tooling in place that checks whether documentation builds locally before accepting any pull request into master that touches documentation.