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

refactor: extract common code from CharmRecipe #585

Merged
merged 4 commits into from
Dec 11, 2024
Merged

Conversation

tigarmo
Copy link
Contributor

@tigarmo tigarmo commented Dec 10, 2024

The code in CharmRecipe is 99% artifact-type-agnostic; generalize it out so that we can provide more artifact types with little new code.

The first commit just "renames" the existing CharmRecipe to _StandardRecipe; the second commit then has the actual changes to generalize the class, with the concrete CharmRecipe implementation.

This class will hold the implementation that is valid for all non-snap recipe
types. For now it's just the previous CharmRecipe - the next commit will do
the generalization work.
Subclasses must provide the name of the artifact (charm, rock, etc) and the
launchpad object that wraps the utility to manipulate recipes.
@tigarmo tigarmo force-pushed the work/refactor-recipe branch from 8b4a06d to 724c648 Compare December 10, 2024 14:18
@tigarmo tigarmo requested review from lengau and a team December 10, 2024 14:33
@tigarmo tigarmo marked this pull request as ready for review December 10, 2024 14:33
Copy link
Contributor

@bepri bepri left a comment

Choose a reason for hiding this comment

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

Looks good! Just one suggestion.

craft_application/launchpad/models/recipe.py Outdated Show resolved Hide resolved
@tigarmo tigarmo added the squash label Dec 10, 2024
@tigarmo tigarmo force-pushed the work/refactor-recipe branch from ab60420 to 8a606f1 Compare December 10, 2024 18:52
@tigarmo tigarmo requested review from lengau and bepri December 10, 2024 18:55
Copy link
Contributor

@bepri bepri left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

Copy link
Contributor

@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.

🚀 🇧🇷 🚀

@lengau lengau merged commit 8357448 into main Dec 11, 2024
9 checks passed
@lengau lengau deleted the work/refactor-recipe branch December 11, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants