Skip to content

{bio}[foss/2015b] NextClip (REVIEW)#2544

Merged
boegel merged 4 commits intoeasybuilders:developfrom
nathanhaigh:NextClip-1.3.1-foss-2015b
Feb 29, 2016
Merged

{bio}[foss/2015b] NextClip (REVIEW)#2544
boegel merged 4 commits intoeasybuilders:developfrom
nathanhaigh:NextClip-1.3.1-foss-2015b

Conversation

@nathanhaigh
Copy link
Copy Markdown
Contributor

No description provided.

@hpcugentbot
Copy link
Copy Markdown

Automatic reply from Jenkins: Can I test this?

@nathanhaigh
Copy link
Copy Markdown
Contributor Author

The script directory contains Perl scripts - should Perl then be added as a dependency?

@boegel
Copy link
Copy Markdown
Member

boegel commented Feb 23, 2016

Jenkins: ok to test

@boegel
Copy link
Copy Markdown
Member

boegel commented Feb 23, 2016

@nathanhaigh: I'd say that depends on how important those scripts are... Are they considered to be a main part of this software, or just helper scripts that happen to be included?

@boegel boegel added this to the v2.7.0 milestone Feb 23, 2016
@hpcugentbot
Copy link
Copy Markdown

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

boegel commented Feb 23, 2016

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
Linux SL 6.7, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.6.6
See https://gist.github.com/e18fdb5dfaf3cb6b70ca for a full test report.

@boegel
Copy link
Copy Markdown
Member

boegel commented Feb 23, 2016

@nathanhaigh: let me know what you'll do w.r.t. the Perl dep, otherwise good to go

@nathanhaigh
Copy link
Copy Markdown
Contributor Author

I'd say they were more like helper scripts. On a slightly closer loo, there are also R scripts there as well as Perl scripts. I wouldn't add them. Also the shebang lines of the Perl scripts ought to be changed from #!/usr/bin/perl to #!/usr/bin/env perl

@boegel
Copy link
Copy Markdown
Member

boegel commented Feb 23, 2016

Changing the script to use /usr/bin/env would be good indeed...

@hpcugentbot
Copy link
Copy Markdown

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

+++ b/scripts/nextclip_plot_graphs.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
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.

hmm, I think this is very uncommon?

1) Added comment and author.
2) Strip superfluous `-w` from perl shebang line.  - the scripts already use the `use warnings;` pragma.
@hpcugentbot
Copy link
Copy Markdown

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

@@ -0,0 +1,51 @@
Fix hardcoding of /usr/bin/perl, use perl available through $PATH
Fix hardcoding of /bin/bash, use bash available through $PATH
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.

drop this comment if you revert those parts of the patch?

@hpcugentbot
Copy link
Copy Markdown

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

boegel commented Feb 29, 2016

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
Linux SL 6.7, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.6.6
See https://gist.github.com/71e965091f9fa6bdecc9 for a full test report.

@boegel
Copy link
Copy Markdown
Member

boegel commented Feb 29, 2016

Thanks @nathanhaigh!

boegel added a commit that referenced this pull request Feb 29, 2016
@boegel boegel merged commit bdce6cd into easybuilders:develop Feb 29, 2016
@nathanhaigh nathanhaigh deleted the NextClip-1.3.1-foss-2015b branch February 29, 2016 23:22
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.

4 participants