-
-
Notifications
You must be signed in to change notification settings - Fork 308
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 builder #2062
base: nightly
Are you sure you want to change the base?
Refactor builder #2062
Conversation
spellcheck is failed because of CHANGELOG? Not related to my change |
don't worry about that :) |
Is there any config that enables everything that I can use to check if it works? |
There's a "kitchen sink" config in the json schema folder in nightly. |
Tried with kitchen sink, other than collection issues as mentioned in #2065 it is working without errors now. |
Can I ask what the "purpose" of this PR is? Does it provide any performance benefit, or is it purely streamlining the code? |
Purely streamlining, unless you write the builder, no one can read a 4k lines monolithic class. It’s really against all kinds of best practice, so I’m just refactoring to make it easy to read and change in the future |
Has anyone reviewed the PR and any feedback? This PR should have no actual logic change, just refactoring only. Also, I'm not python programmer by trade so hopefully I adhere to python best practice in general. |
It will be up to the author and repo owner. |
c13e173
to
f108b79
Compare
16a01f7
to
f1aa2ed
Compare
e4a9ea7
to
9c9e301
Compare
Description
Builder module is too long, by creating a package and breaking it down, I've managed to reduce it to <2000 lines. More still could be done, but it's much more readable now with a still large constructor, but otherwise only core methods.
Issues Fixed or Closed
Type of Change
Please delete options that are not relevant.
Checklist