-
Notifications
You must be signed in to change notification settings - Fork 30
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 small items on the website #362
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.
A couple of little suggestions - thanks @abachma2
|
||
News | ||
---- | ||
|
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.
Should we generate some news? Maybe @abachma2's PhD?
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.
@nuclearkatie had an item about the latest news being from 2018, but I wasn't sure what news items we could add. We could add an announcement about the current NEUP grant, show that there is active development on the code.
source/user/install.rst
Outdated
------------------------------------- | ||
|
||
Try running |Cyclus| for yourself. The result will be a :doc:`database <dbdoc>` named cyclus.sqlite. Use the drop down menu to load the sqlite file into Cyclist for data visualization, or use your favorite sqlite browser to peruse. | ||
Try running |Cyclus| for yourself. The result will be a :doc:`database <dbdoc>` named cyclus.sqlite. Use the drop down menu to | ||
load the sqlite file into Cyclist for data visualization, or use your favorite sqlite browser to peruse. |
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.
Shouldn't we remove reference to Cyclist
here?
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 think this needs an update/rebase, too
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.
Suggested a couple minor changes. I have one additional question about the example setup.py
in this tutorial - why is 'xo'
included as a script? Is it just included as an indication that users can include their own scripts in the package or is actually necessary for this tutorial? I removed this dependency from the Dockerfile in #1735 and want to make sure I'm not breaking anything.
Do we do rebase, or merge with this repo? I think rebase. |
I don't know about that one. @gonuke might have some more insight on what that does for the build. |
Good catches @bennibbelink. Hopefully it all renders correctly now. |
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.
Thanks @abachma2!
|
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.
In that case @abachma2 are you opposed to removing 'xo'
from line 63 of tutorial_py/setup.rst
?
I am always down for removing unneeded dependencies from installation instructions. |
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 good. Are we waiting to include some news items in this PR or should we merge and save that for another PR?
I don't think that's a blocking thing for this PR. I think we have enough other items in #337 that can include a new News item in a later PR. Maybe we can discuss somewhere else what a good news item would be. |
3fb0a8f
to
3360941
Compare
I took the liberty of rebasing this PR. @gonuke does that resolve your requested changes? |
This PR addresses some of the items in #337, such as missing
>
in the tutorial, fixing links to places, updating projects that use Cyclus, listing funding sources for Cyclus, etc. Others came up with some of the items this PR addresses, so definitely welcoming feedback on some of the changes.