Skip to content
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

feat: add uv plugin to charmcraft #2050

Merged
merged 20 commits into from
Dec 20, 2024
Merged

feat: add uv plugin to charmcraft #2050

merged 20 commits into from
Dec 20, 2024

Conversation

bepri
Copy link
Contributor

@bepri bepri commented Dec 17, 2024

Needs canonical/craft-parts#945

Closes #2040.
CRAFT-3816

@bepri bepri force-pushed the work/uv-plugin-add/CRAFT-3816 branch from 1c7b6f2 to 4f47635 Compare December 18, 2024 17:02
@lengau lengau force-pushed the work/uv-plugin-add/CRAFT-3816 branch from 4f47635 to 8e38ee7 Compare December 18, 2024 17:02
@bepri bepri requested a review from tigarmo December 18, 2024 20:10
@bepri bepri marked this pull request as ready for review December 18, 2024 20:10
@bepri bepri requested a review from medubelko as a code owner December 18, 2024 20:10
docs/reference/plugins/uv_plugin.rst Outdated Show resolved Hide resolved
docs/reference/plugins/uv_plugin.rst Outdated Show resolved Hide resolved
docs/reference/plugins/uv_plugin.rst Outdated Show resolved Hide resolved
docs/reference/plugins/uv_plugin.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@medubelko medubelko left a comment

Choose a reason for hiding this comment

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

LGTM.

uv.lock Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiosity, why was this file updated? And how?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely from me running "uv sync" or "uv lock" at some point without telling it to freeze the lock file by mistake. @lengau would you like me to revert that or should we keep it in?

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this was necessary to bump the version of craft-parts - I'm just wondering about the "correct" procedure here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah any time a dependency changes we should update the lock file. uv sync or uv lock should do the minimum changes needed to meet the new dependencies.

Copy link
Contributor

@tigarmo tigarmo left a comment

Choose a reason for hiding this comment

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

very nice!

service_factory.lifecycle.run("stage")

# Check that the part install directory looks correct.
assert (install_path / "src" / "charm.py").read_text() == "# Charm file"
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that this copying of the original source is a charmcraft thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe so - I took this test directly from the existing poetry tests since the two are so similar.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is - the charm isn't necessarily an installable module and needs to be included in the src directory.

Copy link
Collaborator

@lengau lengau left a comment

Choose a reason for hiding this comment

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

🚢 :shipit:

@lengau lengau enabled auto-merge December 19, 2024 21:47
@bepri bepri disabled auto-merge December 19, 2024 21:49
@lengau lengau enabled auto-merge December 19, 2024 22:09
@lengau lengau added this pull request to the merge queue Dec 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Dec 20, 2024
@lengau lengau merged commit 6d0d9ce into main Dec 20, 2024
28 checks passed
@lengau lengau deleted the work/uv-plugin-add/CRAFT-3816 branch December 20, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for the uv plugin
4 participants