Skip to content

Conversation

@tuxette
Copy link
Collaborator

@tuxette tuxette commented Sep 23, 2025

This has been tested in MissingData and it works. However, this involves asking people using validate-ctv to propagate this commit:

d37e0dc

in their repository. In clear: if we directly push the current action in CTV, that will result in errors for all people currently using ctv-validate. So next steps will be:

  1. Incorporate the changes in ctv repository (be careful to change the URL in validate-ctv.yml to reference CTV action and not action in Dirk's repository).
  2. Inform users? @zeileis What do you think? Do we have a way to do it easily?
  3. (only for me but I put it here to not forget) Clean the mess in MissingData repository

Unsolved questions: @zeileis ?

  1. We previously had an option to turn off system settings: I guess that we remove this option?
  2. Do we entirely remove the Author field?

@zeileis
Copy link
Collaborator

zeileis commented Sep 23, 2025

Nathalie, thanks for this. The difference between the validate-ctv/action.yml and the .github/workflows/validate-ctv.yml was (and is) not really clear to me. I was really just guessing and didn't try to read the documentation, sorry if this created extra work for you. Regarding your comments/questions:

  • rocker/r2u setup: Is it not possible to enforce this in the validate-ctv/action.yml? Can it only be specified by the .yml files for the individual workflows? I'm asking because I thought that the rocker/r2u setup fulfilled a similar role as the r-lib/actions/setup-r@v2 that is now in the validate-ctv/action.yml.
  • If this cannot be avoided I would simply push this into the five other repositories (or open a PR) which currently use the validate-ctv action.
  • System settings: I'm not sure what this option is for exactly and when it is useful. If you think it is useful in some settings, feel free to reintroduce it.
  • Author field: I had removed this because GitHub didn't allow it in the workflow .yml but apparently it is ok in the action .yml. In that case adding author: CRAN Task View Initiative would work, I think.

@tuxette
Copy link
Collaborator Author

tuxette commented Sep 23, 2025

Specifying the container in the called file (in a composite action) is not directly possible (I think). If I'm right, we can change this file using something that would look like:

runs:
  using: docker
  image: rocker/r2u:latest
  args:
    - bash
    - -c
    - |
      # --- Session info ---
      Rscript -e 'sessionInfo()'

      apt update -qq && apt install --yes --no-install-recommends pandoc

      Rscript -e '
      md <- paste0(basename(getwd()), ".md")

(etc). Ugly to my tastes but we may try this if you prefer.

@tuxette
Copy link
Collaborator Author

tuxette commented Sep 23, 2025

Well, no, forget this: That won't work because I don't see, using this setting, how to split the different checks to produce several outputs as in the original version. I'll search a bit but I'm not sure there is a better solution than specifying the container in the users' files.

@zeileis
Copy link
Collaborator

zeileis commented Sep 23, 2025

OK, nevermind. It's easy enough to change the workflow in the cran-task-views directories.

And just out of curiosity: What does the r-lib/actions/setup-r@v2 do in addition to the rocker/r2u container?

@tuxette
Copy link
Collaborator Author

tuxette commented Sep 23, 2025

I must remove it (or at least try). It just sets a few environment variables. I don't think we need it and I thought I had already removed it (I wanted to... I must have forgotten). I'll take care of this in a couple of minutes or hours.

@tuxette
Copy link
Collaborator Author

tuxette commented Sep 23, 2025

r-lib/actions/setup-r@v2 removed in commit 7f1b79d

@tuxette
Copy link
Collaborator Author

tuxette commented Sep 23, 2025

Author added in commit 86f9112

@tuxette
Copy link
Collaborator Author

tuxette commented Sep 23, 2025

@zeileis : For me, the lost option is not very important so we can proceed with the rest. I'll push the necessary changes in a distinct branch of CTV and will let you merge it. When it's done, I'll take care of MissingData (so you only have four pull requests to make ;-)).

@zeileis
Copy link
Collaborator

zeileis commented Sep 23, 2025

Great, thanks. Regarding the number of repositories: I counted on you making the changes in MissingData and Omics. And then I will take care of the other five 😅

@zeileis zeileis merged commit 3e5420e into master Sep 24, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants