Skip to content

make additional configopts in PETSc easyconfigs work after uncommenting#8522

Merged
boegel merged 1 commit intoeasybuilders:developfrom
ravage:master
Jul 4, 2019
Merged

make additional configopts in PETSc easyconfigs work after uncommenting#8522
boegel merged 1 commit intoeasybuilders:developfrom
ravage:master

Conversation

@ravage
Copy link
Copy Markdown
Contributor

@ravage ravage commented Jun 29, 2019

After removing comments to enable SLURM batch support the build stops working due to malformed configure options.

@boegel boegel added the bug fix label Jul 4, 2019
@boegel boegel added this to the next release (3.9.3) milestone Jul 4, 2019
@boegel boegel changed the base branch from master to develop July 4, 2019 21:41
@easybuilders easybuilders deleted a comment from boegelbot Jul 4, 2019
@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2019

(close & re-open to retrigger Travis on top of develop)

@boegel boegel closed this Jul 4, 2019
@boegel boegel reopened this Jul 4, 2019
@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2019

@ravage Thanks a lot for your contribution, and sorry that it took us a while to pick up on it...

Travis barfed at you a bit because of failing unit tests, which was caused by a combination of targeting the master branch and an issue we fixed recently in the Travis config (see #8483), it wasn't because of a mistake you made.
I hope that wasn't too confusing for you...

For future contributions, please target our develop branch (see https://easybuild.readthedocs.io/en/latest/Contributing.html for detailed information(, or use our awesome GitHub integration features (see https://easybuild.readthedocs.io/en/latest/Integration_with_GitHub.html#github-new-update-pr).

@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2019

Test report by @boegel
SUCCESS
Build succeeded for 4 out of 4 (4 easyconfigs in this PR)
node3102.skitty.os - Linux centos linux 7.6.1810, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz, Python 2.7.5
See https://gist.github.com/3fc455b400650a388fd30795f04bb2f4 for a full test report.

@ravage
Copy link
Copy Markdown
Contributor Author

ravage commented Jul 4, 2019

@boegel Thank you for picking it up, merging and provide instructions for getting it right next time :)

This was the first time submitting a PR to repositories i'm not familiar with. At first the failure caught me by surprise, but then, noticed it was a problem resolving a Python dependency. And since it was such a small change i didn't make much of it.

In the future will follow the rules described in the documentation you pointed out as i should have done in the first place.

@ravage ravage closed this Jul 4, 2019
@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2019

@ravage Did you mean to close the PR? :)

@ravage
Copy link
Copy Markdown
Contributor Author

ravage commented Jul 4, 2019

@boegel doing damage again 😱? I'll reopen it... it looks like i have no idea of what i'm doing 😂

@ravage ravage reopened this Jul 4, 2019
@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2019

Making mistakes are the best way to learn, don't worry about it.

Only downside is that we'll have to wait for Travis again now, since it'll re-test the PR because of the close/re-open :)

@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2019

Going in, thanks @ravage!

@boegel boegel merged commit c0071f3 into easybuilders:develop Jul 4, 2019
@ravage
Copy link
Copy Markdown
Contributor Author

ravage commented Jul 4, 2019

Sorry 😅 will step away from the keyboard!

@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2019

Ah, apparently not, even Travis is confused I guess... :)

PR is merged now, to be included with the next EasyBuild release (v3.9.3).

Thanks a lot for your contribution, keep 'em coming! 👍

@boegel boegel changed the title Make additional configopts work after uncommenting make additional configopts in PETSc easyconfigs work after uncommenting Jul 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants