Skip to content

Refactoring - Organize functions from core.py into classes #367

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
Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Refactoring - Organize functions from core.py into classes #367

wants to merge 1 commit into from

Conversation

IrvingMg
Copy link
Collaborator

@IrvingMg IrvingMg commented Feb 6, 2025 •
edited
Loading

Fixes / Features

  • Refactoring. Organize functions from core.py into classes.

Testing / Documentation

Testing details.

  • [ y/n ] Tests pass
  • [ y/n ] Appropriate changes to documentation are included in the PR

@IrvingMg IrvingMg force-pushed the refactor-group-classes branch from 2b3de73 to 9e7e27f Compare February 12, 2025 16:13
@IrvingMg IrvingMg force-pushed the refactor-group-classes branch 5 times, most recently from 1565531 to 70b9ac8 Compare February 13, 2025 17:40
@IrvingMg IrvingMg mentioned this pull request Feb 19, 2025
Merged
pawloch00
pawloch00 reviewed Feb 27, 2025
View reviewed changes
src/xpk/commands/cluster.py
add_zone_and_project(args)
GCloudContextManager.add_zone_and_project(args)
cluster_manager = ClusterManager(args, system)
capacity_manager = CapacityManager(args)
Copy link
Collaborator

Choose a reason for hiding this comment

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

May I ask you not to use args in new implementations of classes? Please pass required parameters in constructor.

@IrvingMg IrvingMg force-pushed the refactor-group-classes branch 12 times, most recently from 6faea23 to 4739884 Compare March 5, 2025 12:58
@IrvingMg IrvingMg force-pushed the refactor-group-classes branch from 4739884 to c9d6d36 Compare March 5, 2025 13:02
@IrvingMg
Copy link
Collaborator Author

IrvingMg commented Mar 5, 2025

Splitting this PR into smaller ones:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pawloch00 pawloch00 pawloch00 left review comments

@Obliviour Obliviour Awaiting requested review from Obliviour Obliviour will be requested when the pull request is marked ready for review Obliviour is a code owner

@44past4 44past4 Awaiting requested review from 44past4 44past4 will be requested when the pull request is marked ready for review 44past4 is a code owner

@sharabiani sharabiani Awaiting requested review from sharabiani sharabiani will be requested when the pull request is marked ready for review sharabiani is a code owner

@BluValor BluValor Awaiting requested review from BluValor BluValor will be requested when the pull request is marked ready for review BluValor is a code owner

@gcie gcie Awaiting requested review from gcie gcie will be requested when the pull request is marked ready for review gcie is a code owner

@RoshaniN RoshaniN Awaiting requested review from RoshaniN RoshaniN will be requested when the pull request is marked ready for review RoshaniN is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
release-improvements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@IrvingMg @pawloch00