Add document and style workflows

[arisp99]

This PR adds two new workflows, document and style, which document a package and style the code in a package. Both workflows are triggered by changes to the R/ directory on pushes and PRs. After changes are made, the workflows commit and then push the changes to the branch.

Workflows were tested here: https://github.com/arisp99/testactionspkg.

Closes #434.

[krlmlr]

FYI: I'm working on a similar workflow that runs document() and style() as part of a smoke test. The advantage here is that subsequent jobs (the full test and pkgdown) can use the results of document() and style() (via the ref: argument to actions/checkout).

It's in a toy repo because checks there run so quickly: r-dbi/RKazam#16.

Came here to look for an example of caching. We could consider caching the styler cache (in ~/.cache/R/R.cache) to avoid long delays in large codebases.

[arisp99]

@krlmlr Super neat strategy for running workflows. BTW it seems that an alternative strategy for having jobs run after document() and style() could be to leverage the workflow_run trigger.

I think caching the styler cache is a great idea! We may also want to consider adding more flexibility in customizing styler options (something like how you check the styler options in the DESCRIPTION file). The way the workflow is currently coded, we always call the default options (e.g. style = tidyverse_style). However, I am not sure if this is truly needed for the purposes of providing a simple example workflow.

[krlmlr]

Thanks! Does the workflow_run trigger require a token with special permissions? A monolithic workflow isn't ideal, but local composite actions at least avoid most of the repetition.

[arisp99]

It doesn't look like it! Here are the docs for your reference. I've never actually used it myself so I am not sure if it's the best solution...

[codecov-commenter]

Codecov Report

Merging #460 (d0ab4d0) into v2-branch (d6b812f) will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           v2-branch      #460   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
===========================================
  Files              1         1           
  Lines              1         3    +2     
===========================================
+ Hits               1         3    +2     

Impacted Files	Coverage Δ
R/foo.R	100.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6b812f...d0ab4d0. Read the comment docs.

[lorenzwalthert]

As far as caching goes, the caches are immutable, as mentioned in #55, which I saw when I tried to verify that assumption in lorenzwalthert/styler#22.

What works is something like

      - name: save R.cache location
        id: save-r-cache-location
        run: |
          cat("##[set-output name=r-cache-location;]", R.cache::getCacheRootPath(), "\n", sep = "")
        shell: Rscript {0}
      - name: R.cache cache
        uses: actions/cache@v2
        env:
          cache-name: r-cache
        with:
          path: ${{ steps.save-r-cache-location.outputs.r-cache-location }}
          key: ${{ runner.os }}-${{ env.cache-name }}-${{ github.sha }}
          restore-keys: |
            ${{ runner.os }}-${{ env.cache-name }}-
            ${{ runner.os }}-

[arisp99]

Thanks @lorenzwalthert! I just added the Github SHA as part of the cache path.

[lorenzwalthert]

Also, I don't think only changes in R/ should trigger the action. I'd also include tests/, vignettes/ (and specify filetype explicitly, including ".Rmd", which is not the default).

[arisp99]

Also, I don't think only changes in R/ should trigger the action. I'd also include tests/, vignettes/ (and specify filetype explicitly, including ".Rmd" (which is not the default).

Agreed!

[arisp99]

These paths cover the majority of files but may exclude some given a non-traditional package structure. Would it be better to go by file type instead? For example:
paths: ["**.r", "**.R", "**.rmd", "**.Rmd", "**.rmarkdown", "**.Rmarkdown", "**.Rnw", "**.rnw"]

[lorenzwalthert]

That seems more in line with what we actually want, true. Plus ".Rprofile". Also, I don't know exactly what your use case is but since you seem to already use pre-commit, you can also just run it in the cloud with https://pre-commit.ci (it's not free though for private repos I think) or GitHub Actions to enforce the style, as described in https://github.com/lorenzwalthert/precommit/blob/main/NEWS.md#precommit-v020. That would save us from trouble like

• excluding files.
• identical config for pre-commit hook and CI run, e.g. for scope, file filter etc.
• identical environment (package versions etc.), since hooks with pre-commit.com use {renv} to specify hook dependencies.
• no additional config file to maintain.
• planned support for platforms other than GitHub to become more platform agnostic.
• ...

Maybe we should update the docs of {precommit} to point out better that the CI integration can also be used without a local installation.

Anyways, for the cache, I just realised it's better to not use the hash of the commit as a key, because I don't think the cache mechanism will be able to pick up the latest SHA and use that cache. Instead, it will probably just fall back to some random cache key, or alphabetical order. Hence, I think it's better to make the cache date dependent, with fallbacks to yyyy-mm-dd, yyyy-mm.

[arisp99]

Thanks for bringing up running pre-commit on the cloud. I had looked at that briefly but then decided to just run locally. I will certainly look into it! Happy to make doc suggestions as well if I get confused—pre-commit has been a super useful tool!

That seems more in line with what we actually want, true. Plus ".Rprofile"

No harm in including as well but I can't think of too many workflows where someone would push .Rprofile. I think the only time I have pushed is when using {renv} (which is then a single line of code)...

Anyways, for the cache, I just realised it's better to not use the hash of the commit as a key, because I don't think the cache mechanism will be able to pick up the latest SHA and use that cache. Instead, it will probably just fall back to some random cache key, or alphabetical order. Hence, I think it's better to make the cache date dependent, with fallbacks to yyyy-mm-dd, yyyy-mm.

Did a bit more digging into this. Github says here that:

If there are multiple partial matches for a restore key, the action returns the most recently created cache.

So leveraging the SHA should be fine because it can track the most recently created SHA.

[lorenzwalthert]

So leveraging the SHA should be fine because it can track the most recently created SHA.

Thanks, if I was just capable of reading the docs 😉

[arisp99]

Hahaha no worries 😃

[gaborcsardi]

Thanks all! This will be part of @v2 soon, probably today.

[github-actions]

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue and include a link to this pull request.