Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add the ability to compare two Plan objects with == and create a Plan from a dict #1134

Merged
merged 5 commits into from
Feb 27, 2024

Conversation

tonyandrewmeyer
Copy link
Contributor

This PR adds two minor features to pebble.Plan objects to simplify testing (particularly with Scenario):

  • The ability to compare two pebble.Plan objects, or a Plan with a dict, with == - comparison to any other type of object will still return False.
  • The ability to instantiate a pebble.Plan object from a pebble.PlanDict (or equivalent dict).

This aligns the Plan functionality with Layer objects.

@tonyandrewmeyer tonyandrewmeyer marked this pull request as ready for review February 25, 2024 23:14
@tonyandrewmeyer
Copy link
Contributor Author

The spacing lint errors in pebble.py and docs/conf.py are ones that Ruff wants. This seems to be a change in 0.2.2 (#9266), but I'm not sure why it didn't show up in the PR that bumped ruff to 0.2.2. I'll make the adjustments, since they seem minorly better than noqas.

@benhoyt
Copy link
Collaborator

benhoyt commented Feb 26, 2024

Do you mean the PR that bumped ruff to 0.2.1? (1836df5#diff-ef2cef9f88b4fe09ca3082140e67f5ad34fb65fb6e228f119d3812261ae51449) I wonder if we should pin an exact version for Ruff, if this is going to be a problem (particularly as we have preview turned on).

@tonyandrewmeyer
Copy link
Contributor Author

tonyandrewmeyer commented Feb 26, 2024

Do you mean the PR that bumped ruff to 0.2.1? (1836df5#diff-ef2cef9f88b4fe09ca3082140e67f5ad34fb65fb6e228f119d3812261ae51449) I wonder if we should pin an exact version for Ruff, if this is going to be a problem (particularly as we have preview turned on).

Ha, yes. I remembered it going to 0.2 and assumed it must have been 0.2.2 but the "why did it change" obvious answer is indeed that 😂.

We currently have it ruff~=0.2.1 - I would have thought adding new rules would be a minor bump, not a patch one. Looking over the changelog, 0.2 had breaking changes from 0.1, so it's like semvar shifted to the right 😞.

I'd be fine pinning it to an exact version (we could reconsider at 1.0.0 maybe). In the other hand, if the changes tend to be small, I'm also fine with just dealing with them as they come, since we have a small team, so don't generally have a lot of PRs at once.

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. Agreed overall -- some minor comments and changes requested.

ops/pebble.py Outdated Show resolved Hide resolved
ops/pebble.py Outdated Show resolved Hide resolved
ops/pebble.py Outdated Show resolved Hide resolved
test/test_pebble.py Outdated Show resolved Hide resolved
test/test_pebble.py Outdated Show resolved Hide resolved
test/test_pebble.py Outdated Show resolved Hide resolved
test/test_pebble.py Outdated Show resolved Hide resolved
test/test_pebble.py Outdated Show resolved Hide resolved
@benhoyt
Copy link
Collaborator

benhoyt commented Feb 26, 2024

We currently have it ruff~=0.2.1 - I would have thought adding new rules would be a minor bump, not a patch one. Looking over the changelog, 0.2 had breaking changes from 0.1, so it's like semvar shifted to the right 😞.

Heh, Pyright-style "semver". :-)

I'd be fine pinning it to an exact version (we could reconsider at 1.0.0 maybe). In the other hand, if the changes tend to be small, I'm also fine with just dealing with them as they come, since we have a small team, so don't generally have a lot of PRs at once.

Okay, let's leave as is for now, and we can always reconsider if it keeps being a problem.

@benhoyt benhoyt merged commit 9664fd1 into canonical:main Feb 27, 2024
28 checks passed
@tonyandrewmeyer tonyandrewmeyer deleted the minor-plan-tweaks branch February 27, 2024 20:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants