Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Open
Flamefire wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Make CargoPythonPackage the default class for extensions/components of CargoPythonBundle #3993

Flamefire wants to merge 2 commits into from
+7 −0

Conversation

@Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Nov 18, 2025

(created using eb --new-pr)

Micket
Micket approved these changes Nov 19, 2025
View reviewed changes
Copy link
Contributor

@Micket Micket left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Flamefire
Copy link
Contributor Author

Flamefire commented Nov 19, 2025

Note: This currently doesn't change anything because the only difference of CargoPythonPackage to PythonPackage doesn't matter (yet) for Extensions.
Maybe for the components but there currently would need to be a default easyblock anyway which I haven't seen.
Speaking of it: Should we really unconditionally overwrite the default easyblock in (Cargo)PythonBundle and ignore if it was set by the EC?

@boegel boegel changed the title Make CargoPythonPackage the default class for extensions/components of CargoPythonBundle Make CargoPythonPackage the default class for extensions/components of CargoPythonBundle Nov 19, 2025
@boegel boegel added the change label Nov 19, 2025
@boegel boegel added this to the next release (5.2.0) milestone Nov 19, 2025
@Flamefire Flamefire mentioned this pull request Nov 21, 2025
Open
4 tasks
boegel
boegel reviewed Dec 3, 2025
View reviewed changes
easybuild/easyblocks/generic/cargopythonpackage.py
"""Specifically use the overloaded variant from Cargo as is populates vendored sources with checksums."""
return Cargo.extract_step(self)

def configure_step(self):
Copy link
Member

@boegel boegel Dec 3, 2025

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?

Copy link
Member

@boegel boegel Dec 3, 2025

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?

Copy link
Contributor Author

@Flamefire Flamefire Dec 3, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@boegel boegel boegel left review comments

@Micket Micket Micket approved these changes

Assignees

No one assigned

Labels

change

Projects

None yet

Milestone

next release (5.2.0)

Development

Successfully merging this pull request may close these issues.

3 participants

@Flamefire @Micket @boegel