-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fixes deploy mac and tag release #11017
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes several issues in the deployment script and adds new tag/release workflows.
- Defines
OL_REPO
globally and updates alltag_deploy
calls to use it - Changes how the latest tag is determined and adds
tag
andrelease
subcommands - Adjusts interactive prompts in
deploy_wizard
for more intuitive defaults
Comments suppressed due to low confidence (3)
scripts/deployment/deploy.sh:611
- The new
tag
andrelease
subcommands should be documented in the script’s usage/help section so users know they are available.
elif [ "$1" == "tag" ]; then
scripts/deployment/deploy.sh:568
- The new conditional flow in
deploy_wizard
(skipping deploy and optionally checking sync) isn't covered by automated tests. Consider refactoring for testability or adding tests for all interactive paths.
read -p "[Now] Run openlibrary deploy & audit now? [Y/n]..." answer
scripts/deployment/deploy.sh:406
- [nitpick] Removing the PID from the background-start message makes it harder to trace processes during debugging. Consider restoring the PID output for better visibility.
echo "(started in background)"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One bug, and one spot where some duplicated code's been added. Merge at your discretion, I can't decide how problematic that bug is -- we might want to add at least a warning or a notice. I'm a bit tight on time today so can't fix these rn. We can unblock deploys by having Jim make the PID fix locally, that should fix the deploy if his version of bash also for some reason errors with the -1
index.
for more information, see https://pre-commit.ci
Closes #11019
Fixes deploys
Technical
Testing
Screenshot
Stakeholders