Conversation
43cfc62 to
e559f00
Compare
10ce3e6 to
5ce81c6
Compare
bloodearnest
approved these changes
Sep 16, 2025
Member
There was a problem hiding this comment.
This looks good thanks.
Left a comment about maybe making defaulting to false explicit.
Regards using uv to install just, I am -0 on that idea for two three reasons:
- it means we have to trust the PyPI package author (who AFAICT is not a part of the just development team - in fact there are multiple PyPI just packages). I'd rather consume something direct from upstream if possible, reduce the trusted parties
- upstream will likely stay more up to date as things change
- a project might no use python (so no uv) but still want just
So I would vote for using setup-just. I would probably default this to true?
But anyway, that's another issue/PR!
5ce81c6 to
7980211
Compare
Contributor
Author
|
Thank you Simon, I've reworded the commit message + PR description slightly to mention After discussion, defaulted |
7980211 to
be2a0fe
Compare
If a string comparison is not made, setting `install-just` to anything (not just `true`) will trigger the "Install just" step. For the description of the `install-just` parameter to be accurate, check that the value is exactly `true`.
Similar to the `install-just` option, add an `install-uv` option to install `uv` using astral's `setup-uv` GitHub action. No default value is set here, but the repo-template will be setting this to `true`. Using a commit sha following the docs on [Using third-party actions](https://docs.github.com/en/actions/reference/security/secure-use#using-third-party-actions). Dependabot should be able to recognise that the sha corresponds to a specific version and propose updates. `uv` is capable of installing `just`, but I haven't altered the `just` installation part of the action. We may wish to revisit how `just` is installed later, including using `uv` to install it if `install-uv` is set to `true`. We can also use `setup-just`. For flexibility in the future, I have placed the `uv` installation step before the `just` installation step.
be2a0fe to
b672f42
Compare
Closed
Closed
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
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.
Similar to the
install-justoption, add aninstall-uvoption to installuvusing astral'ssetup-uvGitHub action. No default value is set here, but the repo-template will be setting this totrue(opensafely-core/repo-template#305).Using a commit sha following the docs on Using third-party actions.
Dependabot should be able to recognise that the sha corresponds to a specific version and propose updates.
uvis capable of installingjust12, but I haven't altered thejustinstallation part of the action. We may wish to revisit howjustis installed later, including usinguvto install it ifinstall-uvis set totrue. We can also usesetup-just. For flexibility in the future, I have placed theuvinstallation step before thejustinstallation step.Testing:
install-uvset totrueallows runninguvcommands on CI.install-uvand trying to use auvcommand would lead to failing CI as expected.Footnotes
Slack discussion: uv can install just which can run uv ↩
Slack discussion: Installing
justwithuv tool install rust-just↩