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

Add GHA file updates #20

Merged
merged 22 commits into from
Jun 26, 2024
Merged

Add GHA file updates #20

merged 22 commits into from
Jun 26, 2024

Conversation

cansavvy
Copy link
Contributor

Summary

This has to do with the first goal of streamlining the maintenance of OTTR github actions.

We want as few files to maintain as possible and as few copies of those files to update as possible.

So to that end I am updating these files here to see if they work with quarto OTTR version. These files were originally updated on this repo: jhudsl/OTTR_Template#746 But I'm testing them here.

In the future these files will be updated through OTTR sync and fhdsl/ottr has been added to the sync.yml file in that PR as well to make sure this happens.

Copy link
Contributor

github-actions bot commented May 15, 2024

The broken url errors check is currently being re-run 🏃
Comment updated at 2024-06-26-14:34:49 with changes from 1015deb

Copy link
Contributor

github-actions bot commented May 15, 2024

The spelling errors check is currently being re-run 🏃
Comment updated at 2024-06-26-14:34:49 with changes from 1015deb

Copy link
Contributor

github-actions bot commented May 15, 2024

Re-rendered previews from the latest commit:

* note not all html features will be properly displayed in the "quick preview" but it will give you a rough idea.

Updated at 2024-06-26 with changes from the latest commit 36d8526

@howardbaik
Copy link
Contributor

howardbaik commented May 15, 2024

I tried running the GHA's in this PR from a sub-branch of your branch, called test-gha, and I got a couple errors here and here.

@cansavvy
Copy link
Contributor Author

I tried running the GHA's in this PR from a sub-branch of your branch, called test-gha, and I got a couple errors here and here.

This won't run properly the way you have it here because it is pulling files from main that don't yet have all the changes. Namely it doesn't have the config_automation.yml update for options yet.

@cansavvy
Copy link
Contributor Author

I tried running the GHA's in this PR from a sub-branch of your branch, called test-gha, and I got a couple errors here and here.

This won't run properly the way you have it here because it is pulling files from main that don't yet have all the changes. Namely it doesn't have the config_automation.yml update for options yet.

Scratch that. I'm wrong. I found the fix. Thanks for doing that.

@cansavvy
Copy link
Contributor Author

This looks in order to me. Ready to merge when you are @howardbaek But may need more testing?

@howardbaik
Copy link
Contributor

howardbaik commented May 15, 2024

I just pushed a commit to address this issue.

@howardbaik
Copy link
Contributor

It seems like the GHAs that get triggered in this PR are all passing, but the render-all.yml isn't working: https://github.com/fhdsl/ottr/actions/runs/9102695262/job/25022955047#step:5:22

@cansavvy
Copy link
Contributor Author

It seems like the GHAs that get triggered in this PR are all passing, but the render-all.yml isn't working: https://github.com/fhdsl/ottr/actions/runs/9102695262/job/25022955047#step:5:22

It can't find the docs/no_toc/ files. So I'll investigate where it is saving those files (if at all)

@cansavvy
Copy link
Contributor Author

It seems like the GHAs that get triggered in this PR are all passing, but the render-all.yml isn't working: https://github.com/fhdsl/ottr/actions/runs/9102695262/job/25022955047#step:5:22

After digging into this a bit more its failing because of differences of how the html files for quarto vs bookdown are structured. I'm working on adding functionality to ottrpal so this will work.

@cansavvy
Copy link
Contributor Author

cansavvy commented May 16, 2024

jhudsl/ottrpal#129 needs to be reviewed before this PR can continue to be vetted.

After it is reviewed and merged the ottrpal changes need to be:

  1. Added to the ottr_docker through a rebuild of the docker image (it uses the latest ottrpal on github) using https://github.com/jhudsl/ottr_docker/actions/workflows/manual_dispatch.yml
  2. Updated on CRAN through devtools::release()
  3. Then we can retest the coursera and leanpub screenshot making part of render-all.yml to see if that works.

@howardbaik
Copy link
Contributor

Preparing for update to CRAN in jhudsl/ottrpal#132

@howardbaik
Copy link
Contributor

@cansavvy This still isn't working: https://github.com/fhdsl/OTTR_Quarto/actions/runs/9507156191/job/26206018516

Maybe we are not pulling from the right docker image?

@cansavvy
Copy link
Contributor Author

Ah yes. It should be pulling from jhudsl/base_ottr:main

Good call.

@howardbaik
Copy link
Contributor

I got this error: https://github.com/fhdsl/OTTR_Quarto/actions/runs/9520628868/job/26246328655, so I added quarto to the Docker image in this PR: jhudsl/ottr-docker#20

@howardbaik
Copy link
Contributor

Still not working. It seems like ottrpal::get_chapters() isn't compatible with OTTR Quarto's no_toc/index.html

@cansavvy
Copy link
Contributor Author

Still not working. It seems like ottrpal::get_chapters() isn't compatible with OTTR Quarto's no_toc/index.html

Okay I can dig into this tomorrow.

@cansavvy
Copy link
Contributor Author

From https://github.com/fhdsl/OTTR_Quarto/actions/runs/9617955283/job/26532508939 I could see that the ottr_quarto image wasn't being called and
Screenshot 2024-06-24 at 2 26 17 PM

A separate docker image is being used here. That docker image could either be updated (probably a good plan or we just make this also use the docker image from the config file).

I think its better that I update the ottrpal docker image and we keep as is.

@cansavvy
Copy link
Contributor Author

Pushed update to ottrpal docker image https://github.com/jhudsl/ottr_docker/actions/runs/9650571147/job/26616478289

@cansavvy cansavvy merged commit ac2be88 into main Jun 26, 2024
1 check passed
@cansavvy cansavvy deleted the cansavvy/gha-updates branch June 26, 2024 14:34
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.

2 participants