Guide to Epiverse GitHub actions for new contributors

[adamkucharski]

It would be useful to have a guide somewhere for what contributors should do if GitHub actions fail, or what common reasons are (e.g. to point them to when submitting PR). Currently the risk is that we have a barrier to entry for new contributions, given default interface isn’t always that informative (e.g. exit code below).

Highlighting this contributor guide in {EpiNow2} that @jamesmbaazam pointed to – sounds like an opportunity to collate advice.

[jamesmbaazam]

Here are my initial contributions:

• Because most, if not all, of the workflows run R code, the error is sometimes recognisable and can be fixed locally and pushed (Examples include lintr errors, objects/function/argument not found, etc).
• Sometimes, you get a pak installation error which requires you to re-run the workflow.
• Some errors require you to re-run the workflow as it might be an issue with the Runner or operating system on the Runner.
• If you made changes to the readme.Rmd that requires rendering the readme.md fike, the render-readme workflow will be triggered on the CI to generate it. Currently, it pauses the other workflows, so you have to close the PR and re-open it (ideally with a comment indicating that's why it was closed because GitHub leaves a comment about the PR being closed and it might help in the future to see why it was closed).

[adamkucharski]

Thanks. It would probably be worth having a short guide somewhere, so competent R users (but not regular package developers or experienced in GitHub CI) can contribute without hitting unfamiliar errors or tools. E.g. have the PR template point to a breakdown like the following of what users can run locally to check before submitting a PR? Thoughts @Bisaloo @chartgerink ?

Epiverse-specific packages for checks

remotes::install_github("epiverse-trace/etdev") etc. (as otherwise get error when calling lintr below)

Automated tests

[Insert brief summary of what tests do in our packages] testthat::test_package()

Automated linting checks

[Insert brief summary of what linting does in our packages] lintr::lint_package()

Additional note: linting seems like the biggest friction for new contributors currently, because it follows an opinionated style that may be unfamiliar to some (e.g. linting feedback seems to suggest }else { but } else { cleaner and also valid). And unlike test snaps, code style cannot be automatically updated currently (I think? Although looks like there are some tools out there that can offer automatic style fixes). Perhaps OK as long as contributor gets clear message about why these constraints are in place.

Automated package structure checks

[Insert brief summary of best practice in our packages] devtools::check()

[chogis]

IMO, It is a good idea to have clear contributing guidelines but competent R users might be put off if they think the guidelines are cumbersome.

The path of least resistance is to assume R users don't normally have {remotes, testthat, lintr, devtools} installed on their machines and their main activity on Github is reading/raising issues and forking repos to make private changes.

If we're lucky and they contribute a PR that fails CI/CD we should be gracious enough to ask them if they want us to fix the CI/CD issues or walk them through how to fix it.

[adamkucharski]

Agree we don't want to introduce unnecessary friction, but I'm aware the current set-up might be off putting for domain specialists who work with R but aren't familiar with our pipelines (i.e. they might make a contribution, fail actions, then hesitate to raise with us because it's unclear where the responsibility for a passing PR falls).

Perhaps simplest solution is to point to a list of CI/CD on the PR page by default, and state that experienced users may want to check these themselves, but we also welcome wider contributions and we can support on issues if needed.

[Bisaloo]

@adamkucharski, are you satisfied with #44 as a resolution for this issue or do you have other additional changes you'd suggest?

[adamkucharski]

I think we're now fine to close - thanks for input on PR #44