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

remove unneeded OTTR scripts #228

Merged
merged 4 commits into from
Jan 27, 2025
Merged

remove unneeded OTTR scripts #228

merged 4 commits into from
Jan 27, 2025

Conversation

KatherineCox
Copy link
Collaborator

@KatherineCox KatherineCox commented Jan 23, 2025

I'm about to start working through #227 to get our existing repos up to date, and thought this would be a good time to do a bit of clean up.

Currently the scripts directory contains:

  • AnVIL_Feedback_Script.sh: we actively use, and want to keep syncing
  • git_repo_check.R: no longer in OTTR_Template, functionality now provided by ottrpal
  • make_screenshots.R: no longer in OTTR_Template, functionality now provided by ottr-reports
  • ottr-fy.R: no longer in OTTR_Template, functionality now provided by ottr-reports

This PR:

1. Deletes unnecessary OTTR files

  • Delete files
  • Update sync.yml

2. Moves AnVIL_Feedback_Script.sh

I'm wondering if we want to move AnVIL_Feedback_Script.sh to .github/AnVIL_Feedback_Script.sh? We can then delete the scripts folder since OTTR no longer uses it. That lets us remove some clutter from the top-level directory by hiding away some backend functionality.

  • Move AnVIL_Feedback_Script.sh from /scripts to /.github
  • Delete /scripts
  • Update /.github/workflows/render-all.yml
  • Update /.github/workflows/pull_request.yml
  • Update sync.yml

3. Rerenders book

Adds a newline so that the book will rerender when this is merged to main and we can confirm everything is working as expected.

The pull-request jobs look like they ran without error.

Copy link
Contributor

github-actions bot commented Jan 23, 2025

No spelling errors! 🎉
Comment updated at 2025-01-23-23:18:35 with changes from 18d41cc

Copy link
Contributor

github-actions bot commented Jan 23, 2025

No broken url errors! 🎉
Comment updated at 2025-01-23-23:18:36 with changes from 18d41cc

Copy link
Contributor

github-actions bot commented Jan 23, 2025

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 2025-01-23 with changes from the latest commit 18d41cc

@KatherineCox
Copy link
Collaborator Author

@avahoffman any objections? any additional complication I might have missed? I think pull_request.yml and render-all.yml are the only files that call AnVIL_Feedback_Script.sh?

Copy link
Contributor

@avahoffman avahoffman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Thanks so much for confirming these updates!

@KatherineCox KatherineCox merged commit 3c6a2d3 into main Jan 27, 2025
11 checks passed
@KatherineCox KatherineCox deleted the clean-up-scripts branch January 27, 2025 13:45
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