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

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

ops/charm.py Outdated Show resolved Hide resolved
ops/main.py Outdated Show resolved Hide resolved
ops/main.py Outdated Show resolved Hide resolved
ops/main.py Outdated Show resolved Hide resolved
ops/main.py Outdated Show resolved Hide resolved
ops/testing.py Outdated Show resolved Hide resolved
ops/testing.py Outdated Show resolved Hide resolved
@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).

ops/charm.py Outdated Show resolved Hide resolved
ops/testing.py Outdated Show resolved Hide resolved
ops/testing.py Outdated Show resolved Hide resolved
ops/testing.py Outdated Show resolved Hide resolved
test/test_testing.py Outdated Show resolved Hide resolved
test/test_testing.py Outdated Show resolved Hide resolved
test/test_testing.py Outdated Show resolved Hide resolved
test/test_testing.py Show resolved Hide resolved
@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
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
19 checks passed
@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