Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a SOS Dockerfile script and a README #1044

Merged
merged 7 commits into from
Mar 23, 2022

Conversation

kholland-intel
Copy link
Collaborator

No description provided.

Copy link
Member

@davidozog davidozog left a comment

Choose a reason for hiding this comment

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

Looks very useful @kholland-intel - thanks!

I have just a few suggestions/questions.

scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/Dockerfile Outdated Show resolved Hide resolved
scripts/docker/Dockerfile Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/Dockerfile Outdated Show resolved Hide resolved
Fixed README typos, updated working directory to "/home/sos", etc.
Added OneAPI binaries to the Dockerfile builder's path for OneAPI
compilers
scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
enables the PRRTE Server.
This is mostly used to run SOS tests when the GNI provider has been enabled in
the Libfabric build.
When testing on the GNI provider uncomment out the lines in this portion to
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be better to say the specific line numbers to be uncommented. Same for the previous cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Instead of pointing to a specific line number, I suggested user comment out the code directly under a specific subheading (I felt like the headers were less likely to change than the line numbers). Does this work or do you still prefere that we point them to a specific line number? Examples can be found in the README changes at lines 52 and 73.

scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
scripts/docker/README.md Outdated Show resolved Hide resolved
Also, address grammatical errors in the README
@davidozog
Copy link
Member

Issues #1047 and #1048 manifested here, but are certainly unrelated to this PR.

@davidozog davidozog merged commit f360114 into Sandia-OpenSHMEM:master Mar 23, 2022
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