Fix for to_str in dump method of easyconfig.py#1308
Fix for to_str in dump method of easyconfig.py#1308boegel merged 21 commits intoeasybuilders:developfrom
Conversation
release EasyBuild v2.1.1
|
Automatic reply from Jenkins: Can I test this? |
|
Jenkins: ok to test |
There was a problem hiding this comment.
use force_str=True (don't specify optional arguments unnamed)
but, the whole force_str thing can be dropped anyway (yes, I know I suggested it ;-)), since the return value is only used in a string interpolation context anyway (and it's already part of a tuple, so no need to force it into a string anymore)
|
Refer to this link for build results (access rights to CI server needed): See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1801/console for more details. Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1802/console for more details. Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1803/console for more details. Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do. |
kickstart tests for quote_str/dump
|
Refer to this link for build results (access rights to CI server needed): See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1805/console for more details. Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
There was a problem hiding this comment.
no tabs please, see http://legacy.python.org/dev/peps/pep-0008/#tabs-or-spaces
add this to your ~/.vimrc:
:set expandtab tabstop=4 shiftwidth=4
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
There was a problem hiding this comment.
what is missing in textwrap?
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
There was a problem hiding this comment.
the regex here is kind of flawed... "" would match too, for example, and since [^"]* also matches the empty string, you're not checking the full string anyway...
maybe just go with '^description = ["\']' ?
There was a problem hiding this comment.
also, use 'raw' strings for regexes: r'^description = ["\']'
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
There was a problem hiding this comment.
last remark (for this PR, at least ;-)): rename to include_defined_parameters, and change docstring to
"""
Internal function to include parameters in the dumped easyconfig file which have a non-default value.
"""|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
Fix for to_str in dump method of easyconfig.py
|
@Caylo: looks good now, going in, thanks! Congrats on your first contribution to EasyBuild! ;-) |
Resolves issue #1305.