-
Notifications
You must be signed in to change notification settings - Fork 21
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
Update documentation for using TotalView with Flux #265
Update documentation for using TotalView with Flux #265
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just one small note about a possible unnecessary comma.
jobs/debugging.rst
Outdated
@@ -48,8 +48,8 @@ can be handy when you debug a large-scale job. Please refer to | |||
|
|||
.. _TotalView user guide: https://docs.roguewave.com/en/totalview/current/html/ | |||
|
|||
Exiting TotalView without completing a full run of your code, may not clean up the Flux job. | |||
In that case you will need to cancel the flux job manually. | |||
Exiting TotalView without completing a full run of your code, may not clean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think a comma is needed here after code
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just pushed a couple fixes including dropping that comma!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like @garlick caught this in his fixes below!
27591f4
to
a9de8c5
Compare
jobs/debugging.rst
Outdated
@@ -47,6 +48,16 @@ can be handy when you debug a large-scale job. Please refer to | |||
|
|||
.. _TotalView user guide: https://docs.roguewave.com/en/totalview/current/html/ | |||
|
|||
Exiting TotalView without completing a full run of your code, may not clean up the Flux job. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superfluous comma?
jobs/debugging.rst
Outdated
This code should either be added to the site-wide ``.tvdrc`` file | ||
This code has been added to the site-wide ``.tvdrc`` file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these docs are not site dependent, it seems like the original was better?
jobs/debugging.rst
Outdated
@@ -48,8 +48,8 @@ can be handy when you debug a large-scale job. Please refer to | |||
|
|||
.. _TotalView user guide: https://docs.roguewave.com/en/totalview/current/html/ | |||
|
|||
Exiting TotalView without completing a full run of your code, may not clean up the Flux job. | |||
In that case you will need to cancel the flux job manually. | |||
Exiting TotalView without completing a full run of your code, may not clean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just pushed a couple fixes including dropping that comma!
Thanks! I'll set MWP. |
Add a few updates for the different UIs as well as some info on canceling flux job
Problem: There's some trailing whitespace and long lines in the jobs/debugging.rst doc. Fix up formatting.
Problem: totalview menu options are not formatted properly. Use the :guilabel: role.
Problem: referencing the presumed content of a site wide totalview config file is inappropriate in site independent docs. Restore the earlier language. Also drop a superfluous comma.
a9de8c5
to
bdbd334
Compare
This adds the helpful TotalView doc updates by @suzannepaterno from #251 without the merge commits. I was unable to force push to the other PR.