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

Allow use actions/config/metadata in charmcraft.yaml #1126

Merged
merged 7 commits into from
Jul 7, 2023

Conversation

syu-w
Copy link
Contributor

@syu-w syu-w commented Jun 16, 2023

Allow use actions/config/metadata in charmcraft.yaml when corresponding yaml file does not exist.

Metadata in charmcraft.yaml also must be new keys defined in spec ST087.

CRAFT-1747
CRAFT-1788
CRAFT-1789

@syu-w syu-w force-pushed the move2charmcraft branch 3 times, most recently from be67c80 to 325b211 Compare July 4, 2023 20:46
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.

Wow, this is a lot! 🙂

I've done a first pass here — mostly asking questions, but there are a couple of things that should go into their own refactors.

charmcraft/models/metadata.py Outdated Show resolved Hide resolved
charmcraft/models/metadata.py Show resolved Hide resolved
charmcraft/models/config.py Show resolved Hide resolved
charmcraft/const.py Outdated Show resolved Hide resolved
charmcraft/linters.py Outdated Show resolved Hide resolved
charmcraft/models/charmcraft.py Show resolved Hide resolved
tests/spread/smoketests/metafiles/actions/task.yaml Outdated Show resolved Hide resolved
tests/test_main.py Outdated Show resolved Hide resolved
tests/test_main.py Show resolved Hide resolved
charmcraft/models/config.py Outdated Show resolved Hide resolved
charmcraft/models/config.py Outdated Show resolved Hide resolved
charmcraft/models/config.py 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.

Please consider my suggestions in charmcraft/models/config.py, but whether you implement them or not is up to you. I'm fine either way!

Thanks for all of this! That was a LOT of work!

syu-w added 7 commits July 7, 2023 16:20
New metadata keys can be used in charmcraft.yaml if
metadata.yaml not exists.

tests: fix windows incomplete escape

config: convert undocumented maintainer

models: fix peers specs

pack: bundle does not need metadata.yaml in zip

config: convert new metadata to legacy metadata when export

config: allow bundle without metadata.yaml
New actions keys can be used in charmcraft.yaml if
actions.yaml not exists.
config keys can be used in charmcraft.yaml if
config.yaml not exists.
Use new prepare_*_yaml for tests.
DISPATCH_FILENAME,
HOOKS_DIRNAME,
]
CHARM_FILES = frozenset(
Copy link
Collaborator

Choose a reason for hiding this comment

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

❤️

Copy link
Contributor Author

@syu-w syu-w Jul 7, 2023

Choose a reason for hiding this comment

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

The only downside is some tests are order sensitive when this changed to unordered, had to add a workaround.

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.

I hereby reaffirm my approval! 🎉

@syu-w syu-w merged commit 044b94f into canonical:main Jul 7, 2023
25 of 26 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.

2 participants