Skip to content

unset buildopts/installopts before installing Python extensions#597

Merged
boegel merged 1 commit intoeasybuilders:developfrom
boegel:python_buildopts_exts
Apr 23, 2015
Merged

unset buildopts/installopts before installing Python extensions#597
boegel merged 1 commit intoeasybuilders:developfrom
boegel:python_buildopts_exts

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Apr 22, 2015

No description provided.

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/916/
Test PASSed.

@pforai
Copy link
Copy Markdown
Contributor

pforai commented Apr 22, 2015

Looks good, but could be generalized in future for other stuff where we have extensions like R, Perl, etc.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 22, 2015

I wouldn't generalise it unless it's really needed, and even then I would make it optional by introducing an extra easyconfig parameter to enable/disable it.

For it, only doing it here (where we have a use case for it) should suffice.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 22, 2015

@wpoely86: thoughts on this one?

@rjeschmi
Copy link
Copy Markdown
Contributor

It looks ok, I just wonder whether it indicates there should be a stronger separation between the Lang EB and it's extensions. Probably something to think about, but this looks fine for now.

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.

keep an empty line here?

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.

there's no reason imho

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.

maybe we really should have a style guide. Some easyblocks have a empty line, some don't.

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.

if this is the main thing we're going to worry about, we should consider EB finished ;)

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.

but but but but, we need something to bitch about! How can we consider this to be a true OSS project if we can't bitch about style?

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.

;-)

@wpoely86
Copy link
Copy Markdown
Member

Looks fine IMHO.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 23, 2015

Thanks for the review everyone!

boegel added a commit that referenced this pull request Apr 23, 2015
unset buildopts/installopts before installing Python extensions
@boegel boegel merged commit 8884952 into easybuilders:develop Apr 23, 2015
@boegel boegel deleted the python_buildopts_exts branch April 23, 2015 08:13
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.

5 participants