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

Refactor setup.py #3393

Merged
merged 8 commits into from
Jun 26, 2023
Merged

Refactor setup.py #3393

merged 8 commits into from
Jun 26, 2023

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented May 30, 2023

Issue

Our lengthy setup.py is so difficult to read. It will be more and more complicated when we introduce more feature specs.

How did I mitigate it

Break setup.py into multiple files:

  • pysetup/constants.py: constants
  • pysetup/helpers.py: misc helpers
  • pysetup/md_doc_paths.py: the markdown file paths setting
  • pysetup/spec_builders.py: the SpecBuilder implementations of each fork
  • pysetup/typing.py: the NamedTuple data structures used in building the spec

I didn't move the helper functions that utilize the ruamel.yaml or marko packages, which require installation via setup.py. Despite this, I think this refactoring helps setup.py concentrate on package dependencies management.

TODO:

  • not sure if it works well on pypi installation

Copy link
Collaborator

@mkalinin mkalinin left a comment

Choose a reason for hiding this comment

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

I very much appreciate refactor towards spec modularization allowing to add a new feature in easier way.

IMO, a potential step further is coming to ./pysetup/{feature_name}.py file containing all required definitions named according to some convention, e.g. FeatureNameBuilder, feature_name_md_files, and so on, including a parent phase. Then setup.py can have a list of enabled features and can seek for required information to build a spec for particular feature relying on the convention. And spec editors will have to follow the convention which isn't that bad.

Copy link
Collaborator

@dapplion dapplion 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 a great step in the right direction! Adding features can become less painful in the near future

@hwwhww hwwhww merged commit f1aabcd into dev Jun 26, 2023
@hwwhww hwwhww deleted the refactor-setup-py branch June 26, 2023 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants