-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix: use optional-dependencies as preferred variant to custom dev-dependencies #1590
base: develop
Are you sure you want to change the base?
Conversation
cc @gavindsouza see https://github.com/frappe/frappe/blob/5b718bb60f856229ea5caddf51955d513d7ed96a/pyproject.toml#L99-L135
|
Quality Gate passedIssues Measures |
|
||
pyroject_config = loads(open(pyproject_path).read()) | ||
for key, deps in pyroject_config["project"]["optional-dependencies"].items(): | ||
bench.run(f"{bench.python} -m pip install {quiet_flag} --upgrade {' '.join(deps)}") |
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 suppose this could just be moved into the function that installs the apps instead
bench.run(f"{bench.python} -m pip install {quiet_flag} --upgrade -e './apps/{app}[{', '.join(pyroject_config["project"]["optional-dependencies"])}]'
Let pip worry about the entire env management
Given we are using the standard to define optional dependencies, I wonder if we could just retire bench.dev-dependencies
in favour of the standard.
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 suppose this could just be moved into the function that installs the apps instead
If "install app" can be equated to "install optional deps, as well", then yes, indeed.
retire
tools.bench.dev-dependencies
Yes that makes sense, absolutely!
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.
For development setups, installing optional dependencies should be ok, although if I go through the effort of making separate groups, I would expect more control.
So, I'm not sure automatically installing the different "optional-dependencies" groups would be the best course of action. Passing --install-optional-dependencies=dev,test
sounds like a saner path to take, also adding a warning/info if developer_mode is enabled on bench and optional-dependencies are present in app pyproject.toml.
No description provided.