-
Notifications
You must be signed in to change notification settings - Fork 36
Attempting to add changesets #207
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
4a6f27c to
fd5d451
Compare
adrianlyjak
left a comment
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.
See comments. Additionally, this hasn't replaced the existing publish/release workflow, so it will sort of conflict with the release process until that's resolved
I made a subsequent attempt to fix this up: #208
scripts/new_version.py
Outdated
| @@ -0,0 +1,19 @@ | |||
| import toml # type: ignore [import] | |||
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.
the purpose of workflows-dev is to centralize these misc scripts in a package. Ideally the changesets code would go there too
packages/workflows-dev/package.json
Outdated
| @@ -0,0 +1,14 @@ | |||
| { | |||
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.
this should not be published
package.json
Outdated
| "scripts": { | ||
| "pre-commit-version": "pnpm changeset", | ||
| "version": "pnpm version-workflows-dev && pnpm version-llama-index-workflows && pnpm version-llama-index-utils-workflow", | ||
| "release": "pnpm release-workflows-dev && pnpm release-llama-index-workflows && pnpm release-llama-index-utils-workflow", |
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.
this seems rather roundabout.
- changesets calls release on the root workspace,
- the root workspace calls another script on all packages, manually defined
- each script calls the individual package script
- which calls a shared bash script
You could just jump straight from 1 to 4, and have the script iterate through all packages (or have them hard coded)
scripts/requirements.txt
Outdated
| @@ -0,0 +1,2 @@ | |||
| toml | |||
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.
why are we using a requirements file? We're already using uv for dependency management
scripts/release.sh
Outdated
| echo "Nothing to release" | ||
| exit 0 | ||
| else | ||
| # build and publish llama_cloud_services |
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.
some copy pasta here
| - name: Setup Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.11" |
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.
seems rather old
scripts/test_new_version.py
Outdated
| @@ -0,0 +1,29 @@ | |||
| import toml # type: ignore [import] | |||
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.
this test/validation doesn't seem very useful
* Make llama-index-workflows into a sub package * Attempting to add changesets (#207) --------- Co-authored-by: Clelia (Astra) Bertelli <133636879+AstraBert@users.noreply.github.com>
Attempt to add changesets + related CI actions for versioning and releasing (should be working)