-
Notifications
You must be signed in to change notification settings - Fork 5
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
97 coding standards rebranch #128
Conversation
…_coding_standards_rebranch
arrrggggghhhhh - so where is the preview now. it looks like checks have passed. I thought it was supposed to be processed? |
https://cable--128.org.readthedocs.build/en/128/developer_guide/coding_standards/ Very strange that GitHub can't find the preview deplotment now. I found it by clicking "Show all checks", then "Details" next to the readthedocs entry |
Thanks for the link. OK - so now to address your comments - 1 what do you mean by where are general rules? I thought maybe I had left in a black heading. Do you mean I should include that as a section?
|
BTW - do you know how to see a rendered version of source code? That is the whole point of using MD right? |
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. My first attempt didn't work. I'm still learning how to review.
Well there is this: Also there is the regular github rendering of the branch: |
ok Co-authored-by: Tammas Loughran <[email protected]>
ok Co-authored-by: Tammas Loughran <[email protected]>
Co-authored-by: Tammas Loughran <[email protected]>
FYI: You can also accept changes in batch under the files changed tab of the pull request. |
Co-authored-by: Tammas Loughran <[email protected]>
…ABLE into 97_coding_standards_rebranch
The 1st link - I like the ACTUNG!! Special Warning notice. You must've done this but I cant see it in mine, probably because I edited the same part of the code AND the code blocks don't reflect what I have locally The 2nd link does As there is no timestamp or something on the 1st IDK how to update it |
Co-authored-by: Tammas Loughran <[email protected]>
Co-authored-by: Tammas Loughran <[email protected]>
Co-authored-by: Tammas Loughran <[email protected]>
Co-authored-by: Tammas Loughran <[email protected]>
…ABLE into 97_coding_standards_rebranch
Co-authored-by: Tammas Loughran <[email protected]>
Co-authored-by: Tammas Loughran <[email protected]>
Batch commit of suggestions Co-authored-by: Tammas Loughran <[email protected]>
Minor edits
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.
No more comments
@ccarouge - we're waiting on your approval |
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.
Reviewed
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 Jhan, I have a few comments. You don't have to accept everything. Let me know what you think.
fix Co-authored-by: Claire Carouge <[email protected]>
Co-authored-by: Claire Carouge <[email protected]>
Co-authored-by: Claire Carouge <[email protected]>
Co-authored-by: Claire Carouge <[email protected]>
Co-authored-by: Claire Carouge <[email protected]>
Co-authored-by: Claire Carouge <[email protected]>
Co-authored-by: Claire Carouge <[email protected]>
Co-authored-by: Claire Carouge <[email protected]>
Co-authored-by: Claire Carouge <[email protected]>
Co-authored-by: Claire Carouge <[email protected]>
…ABLE into 97_coding_standards_rebranch
Co-authored-by: Claire Carouge <[email protected]>
done Co-authored-by: Claire Carouge <[email protected]>
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.
accept grammar
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.
clearing XX
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 noticed a few differences between the templates at the end and the naming convention for the subroutine names. Instead of listing the changes, I went ahead and made the changes myself. So we are ready to merge!
CABLE
Description
Add coding standards information to the Developer's Guide. The original branch was corrupted and it was unclear if the automated tests worked correctly. It was easier to restart than try to resolve within the first branch.
Fixes #97
Type of change
Checklist
📚 Documentation preview 📚: https://cable--128.org.readthedocs.build/en/128/