Review README.rst #108
Open
Review README.rst #108
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Owner
fangohr
commented
Mar 20, 2024
fangohr
commented
- triggered by update to Octoput 14.0
- simplify where possible
- triggered by update to Octoput 14.0 - simplify where possible
remove duplicated top heading as well.
was markdown, should be rst. Unrelated to this merge request (but reduces re-runs of actions).
iamashwin99
reviewed
Mar 20, 2024
|
|
||
|
|
||
|
|
||
| Octopus in spack |
There was a problem hiding this comment.
Any particular reason for removing the title?
If you just clone the repo into a folder, a title in the Readme would quickly let you know what the project is about.
Comment on lines
11
to
+13
| Mostly aimed for maintainers of the Octopus package in spack: to test changes to | ||
| the `octopus/spack.py` file before request merges in spack upstream. | ||
| Spack's `package.py file <https://github.com/fangohr/octopus-in-spack/blob/main/spack/package.py>`_ for Octopus, before requesting merges | ||
| in `spack upstream <https://raw.githubusercontent.com/spack/spack/develop/var/spack/repos/builtin/packages/octopus/package.py>`_. |
There was a problem hiding this comment.
Sounds duplicated. How about a reworded and more explanative sentence like
This repository is mostly aimed at testing new features and bug fixes to the octopus spack package.
The `package.py file <https://github.com/fangohr/octopus-in-spack/blob/main/spack/package.py>`_ in this repository is taken from the `spack upstream <https://github.com/spack/spack/blob/develop/var/spack/repos/builtin/packages/octopus/package.py>`_ , which defines the steps involved in setting up octopus and its dependencies.
(points to the file on github instead of raw source)
Or something similar ( git hub doesn't allow adding code suggestions to lines that have been deleted)
| @@ -50,15 +42,19 @@ want to include netcdf support: | |||
|
|
|||
| - ``spack install octopus +netcdf`` | |||
|
|
|||
| Ideally, there are no errors. This should install Octopus 12.2 | |||
| This should install the last Octopus release available in spack. (Use ``spack info octopus`` to see available versions.) | |||
There was a problem hiding this comment.
Latest sounds better than last ( as in there is no more octopus)
Suggested change
| This should install the last Octopus release available in spack. (Use ``spack info octopus`` to see available versions.) | |
| This should install the latest Octopus release available in spack. (Use ``spack info octopus`` to see available versions.) |
|
|
||
| There are further *variants* you can install. For example: | ||
|
|
||
| - ``spack install octopus +netcdf+parmetis+arpack+cgal+pfft+python+likwid+libyaml+elpa+nlopt`` | ||
|
|
||
| To see an overview of available variants, use ``spack show octopus``. |
There was a problem hiding this comment.
Wops, No idea how this typo was sustained. Thanks for the correction
|
|
||
| Octopus in Docker container | ||
| =========================== | ||
| 3. Use octopus (it should be in the ``$PATH``). You can check the octopus version using ``octopus --version``. |
There was a problem hiding this comment.
"It should be in PATH" might make users think they have to do something.
Suggested change
| 3. Use octopus (it should be in the ``$PATH``). You can check the octopus version using ``octopus --version``. | |
| 3. Use octopus (it should have been added to ``$PATH`` by the previous, `load` command). You can check the octopus version using ``octopus --version``. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.