-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Conversation
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. |
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. 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 - 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. 😮💨 |
I came up with a way to test this - it might not be the most sophisticated, but it seems to have worked.
Based on these results, I'm going to merge carpentries/actions#91 and then re-run the checks here. I think it should work! 🤞 |
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. |
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! |
Link to the Migrating lessons from the previous infrastructure workflow page.