Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor rchk action script #428

Merged
merged 1 commit into from
Nov 10, 2021
Merged

refactor rchk action script #428

merged 1 commit into from
Nov 10, 2021

Conversation

randy3k
Copy link
Contributor

@randy3k randy3k commented Nov 7, 2021

Moving from docker based action to composite action. It allows the action to be used in more complicated settings.

For most users,

  rchk:
    runs-on: ubuntu-latest
    container:
      image: rhub/ubuntu-rchk
      options: --user=root
    steps:
    - uses: actions/checkout@v1
    - uses: r-lib/actions/run-rchk@master

For more complicated usage,

  rchk:
     runs-on: ubuntu-latest
     container:
       image: rhub/ubuntu-rchk
       options: --user=root
     steps:
     - uses: actions/checkout@v1
     - uses: r-lib/actions/run-rchk@master
       with:
         setup-only: true
     - uses: r-lib/actions/setup-r-dependencies@v1
       with:
         cache-version: rchk-1
     - uses: r-lib/actions/run-rchk@master
       with:
         run-only: true

As we are moving away from the docker based action, we have to specify the container in the workflow. It also means that it will break current users. A message will be shown to those users and inform them about the change. Fortunately, there are only a handful of consumers, I could send them PRs once this PR is merged.

Additionally, we need to switch back the docker user to root (which is the default) because rhub/ubuntu-rchk changed it to docker. Without it, we won't have the necessary permission to run things.

    container:
      image: rhub/ubuntu-rchk
      options: --user=root

I have tested the action in the package ustring and the workflow could be found here.

close #380

@codecov-commenter
Copy link

codecov-commenter commented Nov 7, 2021

Codecov Report

Merging #428 (6f22f50) into master (15cc43b) will not change coverage.
The diff coverage is n/a.

❗ Current head 6f22f50 differs from pull request most recent head a308987. Consider uploading reports for the commit a308987 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##            master      #428   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            1         1           
=========================================
  Hits             1         1           

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 2a200e6...a308987. Read the comment docs.

@jimhester
Copy link
Member

What do you think of the approach used in #380?

@randy3k
Copy link
Contributor Author

randy3k commented Nov 8, 2021

We cannot use other actions for example setup-r-dependencies if running the docker command directly. The current implementation allows users to use any actions or run any arbitrary code. Additionally, they could choose a different docker image as long as it is based on rhub/ubuntu-rchk.

@jimhester jimhester merged commit fd9a09a into r-lib:master Nov 10, 2021
@jimhester
Copy link
Member

Ok, sounds good, thanks for the PR!

@github-actions
Copy link

github-actions bot commented Nov 5, 2022

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a rchk composite action
3 participants