-
Notifications
You must be signed in to change notification settings - Fork 219
rework function/method signatures in main.py and parallelbuild.py #815
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
rework function/method signatures in main.py and parallelbuild.py #815
Conversation
…follow build_options/build_specs scheme used for EasyBlock/EasyConfig
easybuild/main.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this set again? force = True is set above already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, you're right, I somehow missed that... will clean this up
|
@boegel some minor remarks, but excellent cleanup. now all that's left is to move some functions out of main in new module! |
|
@stdweird: I'll take the opportunity here to move some stuff around. I already moved |
…s_in_parallel function
…h to removed log file in main function
….tools, move process_easyconfig from main to easyconfig.tools
…dules functions to easyconfig.tools module
…ls.systemtools there)
… build_options in function signature
… regtest from main to parallelbuild module, get_easyblock_instance from parallelbuild to easyblock module
|
@stdweird: I finished the moving around of functions out of |
|
@boegel it's an improvement for main, that's true, but easyblock is now 2k+ lines!!!! that's absurd. modules over 1k lines are not readable by any standard imho. |
|
@stweird: By moving stuff out of I understand that you feel that modules should be kept of reasonable size, but at an extreme you can put every function/class in a separate module. I don't think that helps a lot in keeping things organized well... Here are the current largest modules: It doesn't look that bad, imho, especially since 80% of For now, I'd leave it at this. This took quite a bit of effort (several hours over the last couple of days) to get to this point, avoiding circular dependencies between modules and trying to make sure nothing was being broken in the process... I suggest to try and cook up some well defined style guidelines on size of functions, total size of modules and maybe also number of functions per module, before we start breaking up |
…hat returns None on parsing failure, and fix minor issues reported by PyLint
|
fix a couple of minor issues, but nothing really major, so merging this all in, thanks for the review @stdweird |
rework function/method signatures in main.py and parallelbuild.py
rework function/method signatures in main.py and parallelbuild.py to follow
build_options/build_specsscheme used forEasyBlock/EasyConfigclass constructors