-
Notifications
You must be signed in to change notification settings - Fork 40
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
Fix link in DEPENDENCIES.rst
#595
Conversation
Build Status Report - 796f798 - 2024-04-29 14:34:54 -0500Build
|
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.
Hi @bennibbelink. It looks like this change is mostly so that the website page looks cleaner, correct? I suggested removing the indent so that it renders well on Github.
If this is mostly just for website rendering, we may want to take a look at some of the numbering on that page; the numbering restarts in weird places. Some of it may be from this page, but some may be the .rst
file for the website
Co-authored-by: Amanda Bachmann <[email protected]>
I am having trouble fixing the numbering. It seems to be finicky since we're injecting sections of DEPENDENCIES.rst into the middle of a numbered list in |
I was thinking that part might be tricky. If it's not possible for now, I won't let that block this PR from being merged. |
What's the best way to preview how these things end up in the website? I'm guessing it's a lot of temporary local customization???... |
Personally, I copy the contents of this file into the That, or you can change the Makefile to point to this fork and repo for this file, but I've found that doesn't always capture the latest updates to the file for some reason. |
Near the top of the |
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.
Hi @bennibbelink. Thanks for making those changes. The numbering is still a little weird, and I made a comment about that.
DEPENDENCIES.rst
Outdated
Anaconda_ (or miniconda_ for a more lightweight choice) to prepare it for | ||
Cyclus. | ||
|
||
.. website_include_conda_end | ||
|
||
2. Once you have Conda installed, installing Cyclus straightforward. | ||
#. Once you have Conda installed, installing Cyclus straightforward. |
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.
When this renders on GitHub, this line and l.60 both show up with a 1 for the numbering. This may be because of the .. website_include_conda_end
in the middle. If you build the website, does the numbering all look good? If so, then I think that's a bigger priority and should keep it as is.
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 for the reminder. I've modified it now to include all the information needed for the website, and should be considered in conjunction with cyclus.github.com PR #364. The numbering is looking good on my local build 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.
Approving this now, but I'll merge after I look at 364 in the website
To test this in conjunction with cyclus/cyclus.github.com#364, do I need to do an ad-hoc change to the Makefile in cyclus/cyclus.github.com#364 so that it loads this updated version? |
Correct, you need to reference my fork and the |
Let's dive in and see how this comes together on the website |
Closes #594. Needed to add a space between the link text and the brackets. Renders correctly on my local site build.