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.

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

Core: move Utils to folder module #4032

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

beauxq
Copy link
Collaborator

@beauxq beauxq commented Oct 5, 2024

What is this fixing or adding?

maybe let us organize code better, and stop this file from growing as much

How was this tested?

generated, served, and played a game

@github-actions github-actions bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Oct 5, 2024
@black-sliver
Copy link
Member

I'm wondering if this is really the best solution.

What I suggested on Discord a while back is creating a lower case folder (that'd have to be called util, or archipelago/util or archipelago/utils, not utils).
And then change the old Utils.py to import and export all old functionality from the new lower case util module.

@beauxq
Copy link
Collaborator Author

beauxq commented Oct 5, 2024

My inclination is that the best time to switch to lower case is at the same time as making an installable package, rather than the churn of people migrating their import statements twice.

@Exempt-Medic Exempt-Medic added the is: enhancement Issues requesting new features or pull requests implementing new features. label Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants