Skip to content

another batch of minor style fixes#4233

Merged
boegel merged 14 commits intoeasybuilders:developfrom
boegel:style_fixes
Mar 2, 2017
Merged

another batch of minor style fixes#4233
boegel merged 14 commits intoeasybuilders:developfrom
boegel:style_fixes

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Mar 1, 2017

No description provided.

@boegel boegel added this to the 3.1.1 milestone Mar 1, 2017
@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 1, 2017

@wpoely86, @migueldiascosta please review?


dependencies = [
('fftw/3.3.4.5', EXTERNAL_MODULE),
('fftw/3.3.4.3', EXTERNAL_MODULE),
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.

not just a style fix, was the change in version number intended?

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.

oh, nice catch, copy-paste error, will fix

Copy link
Copy Markdown
Member

@migueldiascosta migueldiascosta left a comment

Choose a reason for hiding this comment

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

lgtm

"""
description = """The OBITools programs aims to help you to manipulate various data and sequence files in a
convenient way using the Unix command line interface. They follow the standard Unix interface for command line
program, allowing to chain a set of commands using the pipe mecanism. """
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.

delete whitespace at end of last sentence?

]

configopts = '-DCMAKE_BUILD_TYPE=Release -DEIGEN3_ROOT=$EBROOTEIGEN -DCMAKE_CXX_FLAGS="$LIBLAPACK_MT -DEIGEN_USE_MKL_ALL"'
configopts = "-DCMAKE_BUILD_TYPE=Release -DEIGEN3_ROOT=$EBROOTEIGEN "
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.

single quotes?

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.

I tend to use double quotes for strings that include spaces, unless there's a technical reason not to (e.g. double quotes required in the string itself).

See WIP version of easyconfigs style guide at http://boegel-eb.readthedocs.io/en/easyconfigs_style/Code_style.html#code-style-easyconfigs-string-quotes


buildopts = '-f Makefile.SSE3.PTHREADS.gcc CC="$CC" && mkdir -p %(installdir)s/bin && cp raxmlHPC-PTHREADS-SSE3 %(installdir)s/bin'
buildopts = '-f Makefile.SSE3.PTHREADS.gcc CC="$CC" && '
buildopts += "mkdir -p %(installdir)s/bin && cp raxmlHPC-PTHREADS-SSE3 %(installdir)s/bin"
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.

single<->double quotes on the last lines?

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.

same here: contains spaces, hence "

line above requires use of "..." to ensure bash expansion of $CC, so there I use single quotes for the string

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 2, 2017

Thanks for the thorough review @wpoely86 and @migueldiascosta!

@boegel boegel merged commit e66ff4f into easybuilders:develop Mar 2, 2017
@boegel boegel deleted the style_fixes branch March 2, 2017 09:53
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