Skip to content

3di | Making and reviewing a pull request#6892

Open
keithkirkwood-3di wants to merge 17 commits into
ros2:rollingfrom
keithkirkwood-3di:making-reviewing-a-pr
Open

3di | Making and reviewing a pull request#6892
keithkirkwood-3di wants to merge 17 commits into
ros2:rollingfrom
keithkirkwood-3di:making-reviewing-a-pr

Conversation

@keithkirkwood-3di

@keithkirkwood-3di keithkirkwood-3di commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Description

Create new how-to articles for Making a pull request and Reviewing a pull request under new section Contributing to code.

These draw from several existing articles, refactoring the content and removing from the original location.

There will be an action at the end of the tickets under this issue to clean up, so that we don’t end up with any duplicates or dead ends.

Related to: #6828

Did you use Generative AI?

No

Additional Information

Documentation updates from 3di Information Solutions, agreed with Geoff and Tully.

@skyegalaxy skyegalaxy requested a review from sloretz June 4, 2026 16:41
Comment thread source/The-ROS2-Project/Contributing/Developer-Guide.rst Outdated
Comment thread source/How-To-Guides/Core-maintainer-guide.rst

@kscottz kscottz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better than we have so I'll approve it but I made a couple of minor suggestions.

Comment thread source/The-ROS2-Project/Contributing/Contributing-to-code/Making-a-PR.rst Outdated
Comment thread source/The-ROS2-Project/Contributing/Contributing-to-code/Making-a-PR.rst Outdated
Comment thread source/The-ROS2-Project/Contributing/Contributing-to-code/Making-a-PR.rst Outdated
keithkirkwood-3di and others added 3 commits June 10, 2026 15:54
…ng-a-PR.rst

Co-authored-by: Katherine Scott <katherineAScott@gmail.com>
Signed-off-by: Keith Kirkwood <keith.kirkwood@3di-info.com>
Comment thread source/The-ROS2-Project/Contributing/Contributing-to-code/Reviewing-a-PR.rst Outdated
Comment thread source/The-ROS2-Project/Contributing/Contributing-to-code/Reviewing-a-PR.rst Outdated
Comment thread source/The-ROS2-Project/Contributing/Contributing-to-code/Reviewing-a-PR.rst Outdated
keithkirkwood-3di and others added 5 commits June 17, 2026 12:17
…ewing-a-PR.rst

Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Keith Kirkwood <keith.kirkwood@3di-info.com>
…ewing-a-PR.rst

Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Keith Kirkwood <keith.kirkwood@3di-info.com>
@keithkirkwood-3di

Copy link
Copy Markdown
Contributor Author

@kscottz (cc @tfoote) There are 2 types of issues which keep popping up in our docs builds for the 3di staff:

Could you advise on these?

@kscottz

kscottz commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Just a heads up, I'm at a giant trade show this week and I'm not sure how much time I'll have to look into this. Perhaps @ahcorde can kindly take a look.

I did get a brief chance to look at the first error on Thursday. I couldn't track down where it was introduced but it looks like it may be some sort of intermittent request failure due to bot checking. The AI suggests replacing the offending line with https instead of http may resolve the issue. I did not get a chance to verify that's the actual root cause or resolution. It is just a warning so perhaps we could suppress it for the time being.

The second issue seems to be warning on a deprecated versions of ROS that just cropped up. I don't believe anything was back-ported that far which seems odd. I poked around a little bit and the offending file numbers don't line up. There is a reference to rosdistro at the end of the file that points to this location. The file that is referenced doesn't seem particularly useful to the reader so perhaps we can just remove it and see if it clears up.

@kscottz

kscottz commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Heads up @ottojo @ahcorde @christophebedard.

Regarding the first set of warnings mentioned above could this PR have introduced the error? I was grepping around and saw it in the release notes and it seemed like the potential culprit.

@keithkirkwood-3di I just put in #6907 that should fix the second set of warnings. It seems like it removed the "reference:any" errors but the second set of warnings still remain. The PR doesn't exactly fix the problem but it removes the offending lines.

@keithkirkwood-3di

Copy link
Copy Markdown
Contributor Author

@kscottz @christophebedard Hey folks - thanks for the build investigation and fixes. These have resolved my build errors.

Thanks also for the great input - we've now implemented your feedback on these 2 articles, simplifying it in a few places for clarity and brevity (so as not to put off new contributors).

Take a look and let us know what you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants