-
Notifications
You must be signed in to change notification settings - Fork 8
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
⬆️ [maykinmedia/open-api-framework#93] Security updates #216
base: main
Are you sure you want to change the base?
Conversation
set -ex | ||
|
||
command -v uv || (echo "uv not found on PATH. Install it https://astral.sh/uv" >&2 && exit 1) | ||
|
||
cwd="${PWD}" |
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.
I don't think we need cwd
if we use toplevel
in the pip-compile
commands (as git rev-parse --show-toplevel
returns an absolute path), for example
uv pip-compile \
--output-file "$toplevel/requirements/base.txt" \
"$@" \
"$toplevel/requirements/base.in"
You could also call it something like root_dir
to make the variables purpose more descriptive.
-c ci.txt | ||
-r ci.txt |
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.
Shouldn't this be done in the compile_dependencies
script?
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.
It is like this in the default project and I also think its more correct since if you use pip install -r requirements/dev.in
, it will install everything needed for dev vs before where it would just install those 5 libraries.
Though if you just want to install them, its less convenient but I cant think of a situation where I would.
-c base.txt | ||
-r base.txt |
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 comment above
-c base.txt | ||
-r base.txt |
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.
Also applies here
Fixes maykinmedia/open-api-framework#93 partially
Fixes maykinmedia/open-api-framework#81 partially
Changes