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

link to migration guide #79

Merged
merged 1 commit into from
Jun 14, 2024
Merged

link to migration guide #79

merged 1 commit into from
Jun 14, 2024

Conversation

tobyhodges
Copy link
Member

@ErinBecker
Copy link
Contributor

Thank you for this documentation update @tobyhodges. Unfortunately, I was so late to reviewing this that the check logs are no longer available and there is a failed check. I think closing and reopening this PR will re-trigger the checks.

@ErinBecker ErinBecker closed this Jun 7, 2024
@ErinBecker ErinBecker reopened this Jun 7, 2024
@ErinBecker
Copy link
Contributor

Closing and re-opening did indeed retrigger the checks, but they still failed. According to logs, this seems to be a problem with permissions for the github-actions bot.

Screenshot 2024-06-07 at 4 26 29 PM

Which seems to traceback to the Node.js deprecation errors reported elsewhere and tentatively solved by @tobyhodges in this other neglected PR as well as by @Bisaloo in this PR.

GitHub's instructions for updating to Node.js v20 specifies two required parameters - using which this is covered by carpentries/actions#91, and main, which refers back to the action code. @tobyhodges also requests testing in his PR, but unfortunately I have no idea how to test actions. Need to figure that out . . .

It's too late on a Friday afternoon for me to figure this out right now, so I'm adding near the top of @froggleston and my to-work-on-together list, as this issue is as @tobyhodges and @Bisaloo have pointed out, impacting multiple workflows and has been going on for 6 months now. 😮‍💨

@ErinBecker
Copy link
Contributor

I came up with a way to test this - it might not be the most sophisticated, but it seems to have worked.

  1. I forked both this repo and the actions repo.
  2. I merged use node20 actions#91 into my actions fork.
  3. I copied the changes from this PR (link to migration guide #79) to my workbench fork (Update transition-guide.qmd ErinBecker/workbench#1) and confirmed that checks pass.

Based on these results, I'm going to merge carpentries/actions#91 and then re-run the checks here. I think it should work! 🤞

@froggleston
Copy link
Contributor

I think I've fixed this (767421c) with explicit permissions needing to be set for the deployment step. Now, I don't know why this has not been required up til now (the automated workflow runs seem to work, but the manually/PR based ones like this seem to fail). However, all seems to work now, so this should be safe to merge I think.

@ErinBecker ErinBecker closed this Jun 14, 2024
@ErinBecker ErinBecker reopened this Jun 14, 2024
@froggleston froggleston reopened this Jun 14, 2024
@froggleston froggleston reopened this Jun 14, 2024
@froggleston
Copy link
Contributor

I simply can't get this to build correctly from the PR, but maybe that's because it is a PR from a fork rather than a push to main. I suggest merging this anyway as the change isn't build or deployment related!

@ErinBecker ErinBecker merged commit fd2090b into carpentries:main Jun 14, 2024
0 of 5 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