-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Inline Code Docs with Submodules #5928
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: rolling
Are you sure you want to change the base?
Conversation
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
I think it happens here: https://github.com/ros-infrastructure/ros_buildfarm/blob/ff7c1cf38362d9fecae8ef2b677719e2d8ddc04b/ros_buildfarm/templates/doc/doc_independent_docker_job.xml.em#L96-L104 |
|
Thanks! Have opened a pr to make the clone recursive ros-infrastructure/ros_buildfarm#1102 |
fujitatomoya
left a 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.
If this isn't wanted I can add a block around the specific code in the turtlesim launch file.
i think that this is okay.
i believe this is great enhancement as 1st step, even though it is not perfect yet.
it will be hard to replace all the duplicated files in this repository, but i say the less duplicated files the better for the maintenance.
i would like to hear the feedback about this, atm i this this is good thing to do.
had several minor comments need to be addressed.
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
tfoote
left a 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.
This is actually adding complexity to save on one documentation snippet. Clearly there's more that can be imported, but I want to check the direction we're going.
I'm worried about broader maintainability. This core documentation is great as an entry point. But the broader vision is that people should be going to the primary sources, aka the package docuemntation themselves rather than us importing/copying all the content into here.
I think it would be better for us to focus on how to integrate that external documentation in it's current situation rather than improve our import process. We definitely don't want to get to the point that we're importing all modules. This was the downfall of the wiki. If it gets too big it becomes truly unmaintainable.
We've got an ongoing effort to restructure the documentation at the TGC level. I think that before we make this more complicated, we may want to wait for the results of that strategic review before we make this change.
| [submodule "source/_gitmodules/ros_tutorials"] | ||
| path = source/_gitmodules/ros_tutorials | ||
| url = git@github.com:ros/ros_tutorials.git | ||
| branch = rolling |
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.
This looks like it's going to be a challenge for versioning/backporting
Description
Sets up the infrastructure by replacing a copied file with a submodule which hosted the duplicated file.
Fixes #5839
Did you use Generative AI?
Additional Information
Dependabot I think will also need to be enable in the repo or maybe org settings aswell.
The only difference in the output is it now includes the license information in the docs. If this isn't wanted I can add a block around the specific code in the turtlesim launch file.