Skip to content

Fix packaging to exclude logs and test reports#1544

Merged
boegel merged 13 commits intoeasybuilders:developfrom
rjeschmi:package_logs
Jan 16, 2016
Merged

Fix packaging to exclude logs and test reports#1544
boegel merged 13 commits intoeasybuilders:developfrom
rjeschmi:package_logs

Conversation

@rjeschmi
Copy link
Copy Markdown
Contributor

Also clean up some pylint complaints

@hpcugentbot
Copy link
Copy Markdown

Automatic reply from Jenkins: Can I test this?

Comment thread easybuild/tools/package/utilities.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no need to break the import line here? if it's less than 120 chars, keep it on the same line

Comment thread easybuild/tools/package/utilities.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here, why wrap?

if you need to wrap, don't break the string:

raise EasyBuildError("Selected package naming scheme %s could not be found in %s",
                     sel_pns, avail_pns.keys())

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was a line length issue, both pep8 and pylint complain about it.

If we want to make a standard longer line I'm fine with that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

our code style allows lines up until 120 chars, the default in pep8/pylint is 80 I think (which is a nightmare imho)

@boegel
Copy link
Copy Markdown
Member

boegel commented Jan 13, 2016

Jenkins: ok to test

Comment thread easybuild/tools/package/utilities.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use easyblock.installdir.lstrip(os.pathsep)

you have no idea what you're stripping off with [1:]

@hpcugentbot
Copy link
Copy Markdown

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2490/console for more details).

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.

Comment thread easybuild/tools/package/utilities.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do this single line, with .append:

cmdlist.append('--debug')

@boegel
Copy link
Copy Markdown
Member

boegel commented Jan 13, 2016

@rjeschmi: can we enhance the unit test(s) we have for the packaging support to check that logs/test report are not included?

@boegel boegel added this to the v2.6.0 milestone Jan 13, 2016
@hpcugentbot
Copy link
Copy Markdown

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2491/console for more details).

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants