-
Notifications
You must be signed in to change notification settings - Fork 305
Make CargoPythonPackage the default class for extensions/components of CargoPythonBundle
#3993
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
base: develop
Are you sure you want to change the base?
Make CargoPythonPackage the default class for extensions/components of CargoPythonBundle
#3993
Conversation
Micket
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.
lgtm
|
Note: This currently doesn't change anything because the only difference of CargoPythonPackage to PythonPackage doesn't matter (yet) for Extensions. |
CargoPythonPackage the default class for extensions/components of CargoPythonBundle
| """Specifically use the overloaded variant from Cargo as is populates vendored sources with checksums.""" | ||
| return Cargo.extract_step(self) | ||
|
|
||
| def configure_step(self): |
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.
@Flamefire Can you clarify why this change is required?
Also, aren't we missing a return here?
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.
Looks like that's just the change from another PR that is accidentally included here?
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.
Indeed, the danger if --new-pr. Maybe we marge the other one first so this disappears?
As for the return: We never return anything from configure_step
|
This needs a small fix so it works with e.g. pymatgen-2023.12.18-foss-2023a.eb and others this updates Proposed fix: Convert |
…f CargoPythonBundle
We cannot have multiple sources for extensions, see easybuilders/easybuild-framework#3463
7964863 to
84b798d
Compare
Actually the issue was attempting to add (in this case no) crates to |
(created using
eb --new-pr)