Add source control best practices guide - #6917
Conversation
fujitatomoya
left a comment
fujitatomoya
left a comment
There was a problem hiding this comment.
description looks good to me once the workflow is green. i would like to have 2nd approval before merge.
|
i will start the workflow, so that you can see the failures by lines. |
fujitatomoya
added
the
backport-all
|
@sirishapadmasekhar thanks for the PR, can you take a look at https://docs.ros.org/en/rolling/The-ROS2-Project/Contributing/Contributing-To-ROS-2-Documentation.html? the tests and checks can be done in your local environment, please make sure all tests are passing. |
|
@fujitatomoya Thanks for the review. I updated the document to use one sentence per line and pushed the changes. I also ran the local documentation checks ( |
|
@sirishapadmasekhar thanks for fixing. there is one thing to confirm since you did not use our pull request template, did you use any AI tools such as codex or claunde? if so, can you disclose that information? |
peci1
left a comment
peci1
left a comment
There was a problem hiding this comment.
I've suggested a few improvements.
Apart from them, I have one more high-level observation: what about changing the structure like this?
- intro
- avoid committing generated and temporary files
- sources of generated/temporary/backup files
- colcon workspace artifacts
- IDE configs
- editor temporary/backup files
- OS files (.DS_store)
- gitignore placement options:
- repo gitignore
- global gitignore
- sources of generated/temporary/backup files
- credential helpers
- developing on shared robots
- additional resources
308f76f to
c228f68
Compare
|
Thanks @fujitatomoya and @peci1 for the review and suggestions. I have addressed the requested changes, updated the document structure and sentence wrapping, verified that doc8 passes and make html builds successfully, and added the AI usage disclosure to the PR description. |
|
i would like to have 2nd approval on this before merge. please wait for a bit. |
peci1
left a comment
peci1
left a comment
There was a problem hiding this comment.
Sorry for the utter delay, but here we are.
In general, I like the changes you did. There was some line wrapping and unwrapping, so I guess the current state is wanted?
Apart from that, I have two suggestions and that's it. Feel free to reword them differently.
…ices.rst Co-authored-by: Martin Pecka <peci1@seznam.cz> Signed-off-by: Sirisha Padmasekhar <sirishajsekhar@gmail.com>
…ices.rst Co-authored-by: Martin Pecka <peci1@seznam.cz> Signed-off-by: Sirisha Padmasekhar <sirishajsekhar@gmail.com>
|
Thanks for the review! I have updated both suggested changes @peci1 @fujitatomoya |
peci1
left a comment
peci1
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround, approving!
|
@sirishapadmasekhar github workflow is failing, can you address the failures? |
3 participants
Description
Adds a new Source Control Best Practices guide under the Contributing section.
The guide covers:
Also adds the page to the Contributing documentation navigation.
AI Usage
ChatGPT was used to assist with reviewing and restructuring the documentation.
Fixes #6040