Centralize tool versions and improve build consistency #36
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduce versions.toml as the single source of truth for mdBook and plugin versions, used by both CI and local development scripts. Add cross-platform install scripts (install-tools.sh, install-tools.ps1) to automate tool installation based on versions.toml. Update GitHub Actions workflow to use these scripts and bump action versions. Refactor and expand README with unified setup, build, and contribution instructions. Remove redundant better-code/README.md and minor cleanup in book.toml.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Co-authored-by: sparent <sparent@adobe.com>
|
Cursor Agent can help with this pull request. Just |
Introduces a PR preview system using GitHub Pages, including a new workflow for automatic cleanup of preview deployments when PRs are closed. Updates the deploy workflow to support PR previews, adds documentation for the preview system, and enhances the README with details about PR preview deployments.
…m/stlab/better-code into sean-parent/dependency-management
Enhances the deployment workflow to check for the existence of the gh-pages branch and create it as an orphan branch if it does not exist. This ensures smoother deployments and initializes the branch with a README if needed.
Adds the --set-upstream flag to the git push command for the gh-pages branch, ensuring the local branch tracks the remote. This helps with future pushes and branch management in the deployment workflow.
Contributor
📚 Documentation PreviewYour changes have been deployed to a preview environment: 🔗 Preview URL: https://stlab.github.io/better-code/pr-preview/36/ This preview will be automatically updated with new commits and removed when the PR is closed. Built with commit 4f8583b |
Introduces caching for the Rust toolchain and cargo registry in the deploy GitHub Actions workflow to speed up builds and reduce redundant downloads.
Streamlined the GitHub Actions deploy workflow by removing the pull request preview deployment and related steps. Now, the workflow only builds and deploys the book from the main branch, and pull requests are validated by building the book without deploying. Updated the README to reflect these changes and clarify the new process.
Enhances PowerShell and Bash install scripts to handle 'already installed' errors from cargo install, preventing unnecessary failures. Updates deploy workflow to only run deployment steps on main branch for push or manual dispatch events.
Update install-tools.ps1 and install-tools.sh to recognize both 'already exists in destination' and 'is already installed' messages from cargo. This ensures the scripts correctly detect when a package is already installed, accommodating variations in cargo's output.
Removed custom checks for 'already installed' errors in install-tools.ps1 and install-tools.sh. Now, any non-zero exit code from cargo install will cause the script to exit with an error, streamlining error handling. Updated deploy.yml to cache additional Cargo files for improved build performance.
dabrahams
reviewed
Dec 24, 2025
dabrahams
reviewed
Dec 24, 2025
dabrahams
reviewed
Dec 24, 2025
dabrahams
reviewed
Dec 24, 2025
| id-token: write | ||
|
|
||
| # Allow only one concurrent deployment, skipping runs queued between the run in-progress and latest queued. | ||
| # However, do NOT cancel in-progress runs as we want to allow these production deployments to complete. |
Collaborator
There was a problem hiding this comment.
Why is it good to delete the comments in this file?
dabrahams
reviewed
Dec 24, 2025
| jobs: | ||
| # Build job | ||
| build: | ||
| runs-on: ubuntu-latest |
Collaborator
There was a problem hiding this comment.
If we're going to have powershell scripts in here for running on windows, we need to do the build on Windows too, to test them.
dabrahams
reviewed
Dec 24, 2025
dabrahams
reviewed
Dec 24, 2025
dabrahams
reviewed
Dec 24, 2025
scripts/install-tools.ps1
Outdated
| $ScriptDir = Split-Path -Parent $MyInvocation.MyCommand.Path | ||
| $VersionsFile = Join-Path (Split-Path -Parent $ScriptDir) "versions.toml" | ||
|
|
||
| # Function to parse TOML and extract version |
Collaborator
There was a problem hiding this comment.
The contract documentation in this file is nonexistent or not up to snuff.
dabrahams
reviewed
Dec 24, 2025
scripts/install-tools.sh
Outdated
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| VERSIONS_FILE="${SCRIPT_DIR}/../versions.toml" | ||
|
|
||
| # Function to extract version from TOML file |
Collaborator
There was a problem hiding this comment.
The contract documentation in this file is nonexistent, or not up to snuff.
dabrahams
reviewed
Dec 24, 2025
dabrahams
reviewed
Dec 24, 2025
README.md
Outdated
Comment on lines
26
to
29
|
|
||
| ``` | ||
| cargo install mdbook | ||
| **Important**: To ensure consistency between local development and CI, all tool | ||
| versions are centrally managed in `versions.toml`. Use the provided installation | ||
| scripts to automatically install the correct versions: |
Collaborator
There was a problem hiding this comment.
Suggested change
| ``` | |
| cargo install mdbook | |
| **Important**: To ensure consistency between local development and CI, all tool | |
| versions are centrally managed in `versions.toml`. Use the provided installation | |
| scripts to automatically install the correct versions: |
This doesn't belong in the installation instructions.
Member
Author
There was a problem hiding this comment.
Removed and made a pass to tighten up the README.
dabrahams
reviewed
Dec 24, 2025
README.md
Outdated
Comment on lines
29
to
39
| scripts to automatically install the correct versions: | ||
|
|
||
| **Linux/macOS:** | ||
| ```bash | ||
| ./scripts/install-tools.sh | ||
| ``` | ||
|
|
||
| ## Building the book | ||
| **Windows (PowerShell):** | ||
| ```powershell | ||
| .\scripts\install-tools.ps1 | ||
| ``` |
Collaborator
There was a problem hiding this comment.
Why aren't these steps handled on demand by cargo as part of the build? They are dependencies, right?
Member
Author
There was a problem hiding this comment.
Because cargo doesn't auto-install tools as part of cargo build. I went down this path a few different times and failed.
dabrahams
reviewed
Dec 24, 2025
dabrahams
reviewed
Dec 24, 2025
dabrahams
reviewed
Dec 24, 2025
README.md
Outdated
|
|
||
| Please see the [latest published draft](https://stlab.github.io/better-code/) | ||
| for information about the motivation for this project. | ||
| The mdBook is automatically built and deployed to GitHub Pages using GitHub Actions. |
Collaborator
There was a problem hiding this comment.
Seems redundant with item 4 above.
dabrahams
reviewed
Dec 24, 2025
dabrahams
reviewed
Dec 24, 2025
Collaborator
dabrahams
left a comment
dabrahams
left a comment
There was a problem hiding this comment.
I'm afraid this PR is almost everything I loathe about what AI coding assistants produce. It doesn't look like anything you would create on your own and stand behind.
Replaces grep and sed with awk for extracting tool versions from the versions file, allowing for comments and blank lines between section headers and version keys. This makes the script more robust and reliable.
Co-authored-by: Dave Abrahams <dabrahams@adobe.com>
Removes the PR preview and cleanup workflows, consolidates CI to validate PR builds and deploy only from main. Updates README with streamlined instructions and clarifies the new CI/CD process. Improves documentation in install scripts for clarity and maintainability.
…m/stlab/better-code into sean-parent/dependency-management
Replaces the use of awk's match function with sub to extract the version string in a more POSIX-compliant way. This enhances compatibility across different awk implementations.
Updated install-tools.ps1 to install mdbook-katex with the duktape backend on Windows, providing user guidance about limited functionality and alternative installation. Added explanatory comments to versions.toml regarding mdbook-katex installation on Windows.
Deleted the O(N) complexity comment from both install-tools.ps1 and install-tools.sh for clarity, as it was unnecessary for script documentation.
Replaces versions.toml with versions.txt for managing tool versions. Updates CI workflows, install scripts, and documentation to reference versions.txt and simplify version parsing logic. This change streamlines version management and improves cross-platform compatibility.
Member
Author
|
I tried to tighten everything up. I moved to a key=value format for the versions instead of a toml file to avoid messy regex parsing in the scripts (an alternative would be to add a dependency on python but I hate managing python installs). This gets rid of a lot of the "over engineered" feel for the current limited use. |
Refactors the plugin installation loop to use process substitution, ensuring the while loop reads from the filtered list of plugins. This improves compatibility and prevents issues with subshell variable scope.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce versions.toml as the single source of truth for mdBook and plugin versions, used by both CI and local development scripts. Add cross-platform install scripts (install-tools.sh, install-tools.ps1) to automate tool installation based on versions.toml. Update GitHub Actions workflow to use these scripts and bump action versions. Refactor and expand README with unified setup, build, and contribution instructions. Remove redundant better-code/README.md and minor cleanup in book.toml.
See issue #35.
Note
versions.txtas single source of truth for mdBook and plugins with cross-platform installersscripts/install-tools.shandscripts/install-tools.ps1.Build mdBookworkflow (.github/workflows/build.yml) with Ubuntu/Windows matrix, cargo cache, and artifact upload; new PR validation workflowbuild-pr.yml.deploy.ymlto consume the build artifact, configure Pages, and deploy with explicit permissions.README.md; removesbetter-code/README.md; updatesbetter-code/book.toml(addssite-url,cname, enables search/print); adds.tool-versionsfor asdf.Written by Cursor Bugbot for commit 39cfff9. This will update automatically on new commits. Configure here.