Skip to content

implement support for --modules-header#1558

Merged
boegel merged 3 commits intoeasybuilders:developfrom
boegel:modules_header
Jan 21, 2016
Merged

implement support for --modules-header#1558
boegel merged 3 commits intoeasybuilders:developfrom
boegel:modules_header

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Jan 20, 2016

@boegel boegel added this to the v2.6.0 milestone Jan 20, 2016
@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jan 20, 2016

@wpoely86: please review?

@hpcugentbot
Copy link
Copy Markdown

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2527/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.

@hpcugentbot
Copy link
Copy Markdown

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2530/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.

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.

why the not None? if it's empty, that's also trouble?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the default value is None; if it's empty, you're doing something weird (e.g. use eb --modules-header='', and you deserve to know you did something wrong?

You will get a proper error via read_file in that case, which is certainly better than just ignoring this and leaving people wondering why it doesn't work.

@wpoely86
Copy link
Copy Markdown
Member

lgtm

@hpcugentbot
Copy link
Copy Markdown

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2541/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.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jan 21, 2016

Thanks for the review @wpoely86!

boegel added a commit that referenced this pull request Jan 21, 2016
implement support for --modules-header
@boegel boegel merged commit 5063971 into easybuilders:develop Jan 21, 2016
@boegel boegel deleted the modules_header branch January 21, 2016 11:07
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