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: go-framework init profile and extension #1800

Merged
merged 13 commits into from
Aug 27, 2024

Conversation

javierdelapuente
Copy link
Contributor

@javierdelapuente javierdelapuente commented Aug 8, 2024

This PR implements the spec ISD153, "12-Factor Go Support" for charmcraft.

For that, a new init profile called "go-framework" and a new go extension, called "go-framework" are needed. These additions are similar to the flask and django init profiles and extensions.

For the Go profile, the base 24.04 is used.

Code in extensions is refactored, so all 12 factor extensions, that are similar, inherit from _TwelveFactorBase.

@javierdelapuente javierdelapuente force-pushed the go-extension branch 2 times, most recently from 4acc1a3 to 4b1d658 Compare August 8, 2024 14:04
@javierdelapuente javierdelapuente changed the title go-framework init profile and extension Add go-framework init profile and extension Aug 12, 2024
@javierdelapuente javierdelapuente force-pushed the go-extension branch 2 times, most recently from 2a60cc7 to 6feef2c Compare August 14, 2024 10:22
@javierdelapuente javierdelapuente changed the title Add go-framework init profile and extension feat: django-framework extension Aug 14, 2024
@javierdelapuente javierdelapuente changed the title feat: django-framework extension feat: go-framework extension Aug 14, 2024
@javierdelapuente javierdelapuente changed the title feat: go-framework extension feat: go-framework init profile and extension Aug 14, 2024
@javierdelapuente javierdelapuente marked this pull request as ready for review August 14, 2024 13:15
charmcraft/extensions/__init__.py Outdated Show resolved Hide resolved
charmcraft/extensions/twelvefactor.py Outdated Show resolved Hide resolved
charmcraft/extensions/twelvefactor.py Outdated Show resolved Hide resolved
charmcraft/templates/init-go-framework/charmcraft.yaml.j2 Outdated Show resolved Hide resolved
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.

Looks good, thanks!

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.

removing my approval so we fix the supported bases

@javierdelapuente
Copy link
Contributor Author

removing my approval so we fix the supported bases

Corrected to base 24.04 for go and commented all platforms except amd64

arm64:
ppc64el:
s390x:
# arm64:
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps include a note explaining what should be done including which command to run to check the architecture, e.g., uname -m and how to interpret the command output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Added comment with dpkg --print-architecture command, as that is the one giving the correct name (debian architecture names are used)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Added comment with dpkg --print-architecture command, as that is the one giving the correct name (debian architecture names are used)

@lengau lengau added this pull request to the merge queue Aug 27, 2024
Merged via the queue into canonical:main with commit b8d6ecd Aug 27, 2024
20 checks passed
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.

4 participants