Skip to content

{viz} Xmipp (REVIEW)#581

Merged
boegel merged 25 commits intoeasybuilders:developfrom
JensTimmerman:xmipp
Mar 24, 2015
Merged

{viz} Xmipp (REVIEW)#581
boegel merged 25 commits intoeasybuilders:developfrom
JensTimmerman:xmipp

Conversation

@JensTimmerman
Copy link
Copy Markdown

@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/854/
Test PASSed.

@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/855/
Test PASSed.

Comment thread easybuild/easyblocks/x/xmipp.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.

don't use self.__class__, since this will cause havoc for easyblocks deriving from EB_Xmipp

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

right, it breaks inheratance, I forgot why I didn't recommend people to use this :p

@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 17, 2015

@JensTimmerman: fully reviewed, looks pretty clean (after fixing the remarks)

major remark is to not blindly assume deps you require the install root for are loaded, make sure they are by checking that get_software_root is not None

Comment thread easybuild/easyblocks/x/xmipp.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.

try:
    os.chdir(self.cfg['start_dir'])
except OSError as err:
    self.log.error("Failed to change back to %s: %s" % (self.cfg['start_dir'], err))

@JensTimmerman
Copy link
Copy Markdown
Author

further cleaned up, more remarks?

@JensTimmerman
Copy link
Copy Markdown
Author

Test result

Build succeeded for 1 out of 1

Overview of tested easyconfigs (in order)

  • SUCCESS xmipp.eb

Time info

  • start: Wed, 18 Mar 2015 16:41:06 +0000 (UTC)
  • end: Wed, 18 Mar 2015 16:43:49 +0000 (UTC)

@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/875/
Test PASSed.

@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/876/
Test PASSed.

@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/877/
Test PASSed.

@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/878/
Test PASSed.

@JensTimmerman
Copy link
Copy Markdown
Author

@boegel any more remarks?

@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/883/
Test PASSed.

@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 24, 2015

tested with easybuilders/easybuild-easyconfigs#1489, works like a charm, looking good, so going in, thanks @JensTimmerman!

boegel added a commit that referenced this pull request Mar 24, 2015
@boegel boegel merged commit eb988ed into easybuilders:develop Mar 24, 2015
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