-
Notifications
You must be signed in to change notification settings - Fork 8
Update documentation #22
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
base: master
Are you sure you want to change the base?
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.
Thanks for tackling this @abachma2 - I noted a few places that might need improvement (and possibly indicate a need for similar improvements on Cycamore?)
CONTRIBUTING.rst
Outdated
:: | ||
.../cyclus_dir/$ git branch work | ||
.../cyclus_dir/$ git push origin work | ||
.../recycle_dir/$ git branch work |
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.
(here & elsewhere) unless we instruct the user specifically, this is not the name of the directory that git will create
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.
You're talking about the recycle_dir
part of this text? Or the branch names? I think this was pulled from Cycamore so the document there probably has this text 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.
recycle_dir
- we probably wrote the documentation this way to be clear that it was a directory, but a standard git clone
will not create a directory with this name.
We can either give more specific guidance on the git command, or refer to more standard directory names.
I expect this is also flawed on the Cycamore CONTRIBUTING doc
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 added something in to talk about this. 99% chance this language is also in the Cyclus docs, and 100% chance this is in the Cycamore docs.
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'll also note that while I was updated fuelcycle.org @dean-krueger noted that he found the inclusion of the directory in the prompt confusing and worried that some might cut/paste the line that included the directory name. I then discovered that Sphinx will do syntax-based formatting, but only if we do NOT include the directory name. So maybe we take out the directory name entirely?
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.
Updated this file to remove the directory name. How would we use the Sphinx syntax-based formatting for this directory? Or is that something the other repos will use and we should just be consistent across the orgs?
Co-authored-by: Paul Wilson <[email protected]>
I thought we'd wait until cyclus/cyclus#1882 was merged and then updating the CONTRIBUTING here accordingly |
The CONTRIBUTING document here is updated according to the changes made to the Cyclus one in cyclus/cyclus#1882. Ready for another review @gonuke @dean |
|
||
* **DO NOT** rebase any commits that have been pulled/pushed anywhere else other | ||
than your own fork (especially if those commits have been integrated into the | ||
blessed repository). You should NEVER rebase commits that are a part 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.
I've seen in other parts that the term "blessed repository" is being replaced with "mainline main
" or something similar. Just in case that was intentional, wanted to point out that it was used 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.
That is probably a good thing to edit, to make sure it's consistent with the fit hub links we provide.
:: | ||
|
||
$ git chekout main | ||
$ git pull upstream main |
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 maybe missed it, but since this tutorial is seemingly geared towards people who are unfamiliar with Git, should there also be a step where you tell people to set their upstream to the correct place? I think this is the first time I've seen upstream
here. Maybe it does it automatically somehow and I didn't know that?
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.
That is a good idea.
|
||
Additional modules for the Cyclus nuclear fuel cycle simulator from the | ||
Additional archetypes for the Cyclus nuclear fuel cycle simulator from the | ||
University of Wisconsin - Madison are intended to support innovative |
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.
Are these archetypes from UW-Madison? Maybe I'm misreading this, but I thought all the Recycle stuff was from UIUC/Argonne at the moment?
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.
Honestly, I'm not sure. It might just be carryover from Cycamore and I didn't catch it. Probably best to just remove an organization.
#. Fork Cycamore repository, | ||
#. Fork Recycle repository, | ||
|
||
#. Create a working branch on you fork from the ``master`` branch, |
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.
master
--> main
?
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.
Maybe recycle uses master still, but I thought I saw a bunch of other places where this change was made.
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.
Recycle does still use master. I want to make that change on this repo, probably once these changes are merged. So let's make sure they all say main.
#. Implement your modification of the Recycle source code, | ||
|
||
#. Submit a Pull request into ``recyle/master`` branch, | ||
#. Submit a Pull request into ``recycle/master`` branch, |
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.
master
--> main
again?
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.
See above comment.
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 gave it a read-through and flagged some places where I thought I found things. Feel free to ignore if you like though!
This PR addresses #20 by updating some of the basic documentation from saying "Cycamore" to saying "Recycle" to be reflective of the repository. I did not make any updates to documentation related to actual use or purpose of the contents of the repository.
Questions:
CONTRIBUTING
reference the procedure for Cyclus and Cycamore releases, but I'm not sure if that still applies to this repo.