Skip to content

Conversation

lgarber-akamai
Copy link
Contributor

@lgarber-akamai lgarber-akamai commented Apr 19, 2023

📝 Description

This change moves all top-level methods (LinodeClient.nodebalancers(...), etc.) to groups to ensure consistency with the rest of the project. Additionally, alias methods have been created to prevent introducing any breaking changes and to ensure backwards compatibility.

Top-level list methods are now implemented as __call__(...) methods for group classes, and create functions have been moved to create(...) methods inside of each group.

These groups have all been moved to a separate groups package which significantly reduces the size of the linode_client package.

This change also introduces a few fixes for documentation issues:

  • The sidebar width has been slightly increased to prevent content on the sidebar from overlapping with page content
  • The page width has been slightly increased to improve readability
  • Groups that were previously missing from the documentation have since been added

✔️ How to Test

Testing refactor changes:

tox

Testing docs changes:

sphinx-autobuild docs docs/build

@lgarber-akamai lgarber-akamai requested a review from a team April 19, 2023 19:24
@lgarber-akamai lgarber-akamai changed the title new: Move all top-level methods to groups and correct documentation proposal: Move all top-level methods to groups and correct documentation Apr 19, 2023
@lgarber-akamai
Copy link
Contributor Author

lgarber-akamai commented Apr 19, 2023

I just pushed up an implementation I'm much happier with. I'd love to hear some thoughts 🙂

Copy link
Contributor

@ezilber-akamai-zz ezilber-akamai-zz left a comment

Choose a reason for hiding this comment

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

Tests are passing locally! I think this is a much better structure for the project. It's less clunky and much easier to track down where changes should go. Good work!

Copy link
Member

@zliang-akamai zliang-akamai left a comment

Choose a reason for hiding this comment

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

Nice refactor!

Copy link
Contributor

@yec-akamai yec-akamai left a comment

Choose a reason for hiding this comment

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

Great work! Once your change merged, I'll rebase it and refactor the obj group that I'm working on.

@lgarber-akamai
Copy link
Contributor Author

Thanks everyone!

@lgarber-akamai lgarber-akamai merged commit 975438a into linode:dev Apr 21, 2023
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