-
Notifications
You must be signed in to change notification settings - Fork 38
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
Updating BCs Documentation #250
Conversation
Just a reminder for our email conversation, I haven't fully proofread the documentation, so please excuse any typos/left over self-notes. |
Job Documentation on df3868b wanted to post the following: View the site here This comment will be updated on new commits. |
Co-authored-by: Grayson Gall <66559200+gsgall@users.noreply.github.com>
Job Documentation, step Sync to remote on 0148c94 wanted to post the following: View the site here This comment will be updated on new commits. |
Co-authored-by: Grayson Gall <66559200+gsgall@users.noreply.github.com>
Job Precheck, step Clang format on 77376df wanted to post the following: Your code requires style changes. A patch was auto generated and copied here
Alternatively, with your repository up to date and in the top level of your repository:
|
@gsgall @cticenhour This PR is ready for another review. There are some unresolved conversations that focuses on formatting, but nothing too big. |
Thank you @csdechant! I should be able to give this a review Thursday or Friday this week. This weekend at the latest. |
b302c5e
to
99448c9
Compare
@cticenhour Your edits to CIVET seemed to work, as this branch fails precheck due to not referencing an issue number. I will edit the last commit message to reference issue #260 |
99448c9
to
6bb926c
Compare
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.
Sorry for the delay on my review, took longer than expected. Overall great work on addressing the changes @csdechant! I found a few more common typos. A lot of these are just the same sort of typos that were addressed in the previous review. But I think we're almost there!
Co-authored-by: Grayson Gall <66559200+gsgall@users.noreply.github.com>
Co-authored-by: Grayson Gall <66559200+gsgall@users.noreply.github.com>
@gsgall That should be all the new edits. If there is nothing else, I think this PR is ready to go. |
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 to me. Thank you @csdechant. I think we just need @cticenhour to do a review since I don't have merge permission.
Yes, you still only have Triage access for now. Casey and I talked about changing your role for the coming up thesis PRs, but I technically can change you to have Write or Admin access right now. @cticenhour did you have any concerns about changing Grayson's role and having him push this PR? I personal think he should be changed to Admin now. |
I think @gsgall did a great job on reviews here, and have no problem with him getting Admin rights. Since Zapdos efforts will begin to feed into the FENIX project soon, we'll need more reviewers available with high review standards! |
@gsgall you should official have Admin rights now. This PR is showing all green on my end, indicating GitHub accepts the role changes for this PR. |
This PR is part of a continuing effort update the Zapdos documentation. In particular, this PR focuses on the documentation for the Boundary Conditions (BC) objects.