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.

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

Support reading charmcraft.yaml in testing harness #977

Merged
merged 2 commits into from
Jul 24, 2023
Merged

Support reading charmcraft.yaml in testing harness #977

merged 2 commits into from
Jul 24, 2023

Conversation

syu-w
Copy link
Contributor

@syu-w syu-w commented Jul 18, 2023 •
edited by benhoyt
Loading

Try read unified charmcraft.yaml first, that may included metadata, actions, and config. Ignore metadata.yaml, actions.yaml, config.yaml if they are exists in charmcraft.yaml

Fixes #969

@syu-w syu-w force-pushed the charmcraft_yaml branch from 3cb70d8 to d407fb0 Compare July 18, 2023 19:51
@benhoyt benhoyt mentioned this pull request Jul 19, 2023
Closed
benhoyt
benhoyt requested changes Jul 19, 2023
View reviewed changes
Copy link
Collaborator

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! I've added a few comments/questions.

@syu-w syu-w force-pushed the charmcraft_yaml branch from d407fb0 to 4238afb Compare July 19, 2023 19:46
@syu-w
Copy link
Contributor Author

syu-w commented Jul 19, 2023 •
edited
Loading

I removed the change in main.py that doesn't look like it's currently going to be used. ops in charm will still have separate yaml files as for now. Improved import logic, added from_dict and some tests.

@syu-w syu-w requested a review from benhoyt July 19, 2023 19:57
benhoyt
benhoyt reviewed Jul 21, 2023
View reviewed changes
Copy link
Collaborator

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

This is looking very close, thanks. A few nit/style comments and one comment that I don't think from_dict is actually needed (just use the initialiser).

@syu-w
Support read charmcraft.yaml
304bbb1 
Try read unified charmcraft.yaml first, that may included metadata,
actions, and config. Ignore metadata.yaml, actions.yaml, config.yaml if
they are exists in charmcraft.yaml
@syu-w syu-w force-pushed the charmcraft_yaml branch from 4238afb to 304bbb1 Compare July 21, 2023 13:32
@syu-w
Copy link
Contributor Author

syu-w commented Jul 21, 2023

Thanks for the comments, updated.

@benhoyt benhoyt changed the title Support read charmcraft.yaml Support reading charmcraft.yaml in testing harness Jul 24, 2023
@benhoyt benhoyt merged commit 7da07cb into canonical:main Jul 24, 2023
@benhoyt benhoyt mentioned this pull request Jan 3, 2024
Merged
xtrusia pushed a commit to xtrusia/operator that referenced this pull request Jul 11, 2024
@syu-w @xtrusia
Support read charmcraft.yaml (canonical#977)
b7a5b06 
Try read unified charmcraft.yaml first, that may included metadata,
actions, and config. Ignore metadata.yaml, actions.yaml, config.yaml if
they are exists in charmcraft.yaml

(cherry picked from commit 7da07cb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benhoyt benhoyt benhoyt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support unified charmcraft.yaml
2 participants
@syu-w @benhoyt