Skip to content

Conversation

@dpshelio
Copy link
Contributor

This adds a docker container 🐋 for the old version running on 16.04 (now the same than Travis!).

Here we've also updated the readme and Travis to be homogenous with the docker. The installation instructions have been brought inside the repository to keep it in sync with what's currently there (and not pointed to the wiki as that becomes misleading with different versions).

@dpshelio dpshelio requested a review from giordano August 19, 2019 14:15
Copy link

@giordano giordano left a comment

Choose a reason for hiding this comment

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

I could build and successfully use the docker container by following the instructions in the README.md. I left a minor comment about some paths mentioned in that file, though.

I wonder whether it makes sense to move to or duplicate in README.md the test installation instructions that are in INSTALL.md. Another option is point the user to those instructions from the README.md. They're useful, but they're not immediately discoverable from the README.md if the user used the Docker container.

@dpshelio dpshelio self-assigned this Aug 27, 2019
It also "cleans" the travis script by externalising some of the installation
steps as scripts under `.ci`.
So the only need for sudo is to run `make install`
When building petsc it may download the dependencies on a place
that's not being cached properly. Hopefully, running make will
download them again
Copy link
Contributor Author

@dpshelio dpshelio left a comment

Choose a reason for hiding this comment

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

@Jimbles this is an "important" change. I've simply done it so we get colours when opening the configuration files. Could you check whether I'm not missing anything?

This refers to that last commit!

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