Skip to content
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

Update Visualizing ROS 2 data with Foxglove #4620

Merged
merged 13 commits into from
Nov 11, 2024

Conversation

banisadr
Copy link
Contributor

The current how-to guide for Foxglove with ROS 2 is outdated and contains multiple factual inaccuracies. Updating to correct.

@banisadr banisadr marked this pull request as ready for review August 30, 2024 02:48
@banisadr
Copy link
Contributor Author

@clalancette @audrow could I get a review when you have a chance?

@defunctzombie
Copy link

cc @kscottz

@kscottz
Copy link
Collaborator

kscottz commented Sep 17, 2024

I hit the CI for you and did a cursory review. I didn't see anything wrong with the update itself.

One thing that I think might need discussion is the location in the document tree. We've been moving most of the third-party project documents to this location. I believe the original file may pre-date that recedent. If we're updating this doc we should update the location as well. @clalancette @fujitatomoya do you have any thoughts with regard to this?

@fujitatomoya
Copy link
Collaborator

One thing that I think might need discussion is the location in the document tree. We've been moving most of the third-party project documents to this location.

i think this is good idea.

@banisadr btw, awesome gundom picture, i love that.

@clalancette
Copy link
Contributor

One thing that I think might need discussion is the location in the document tree. We've been moving most of the third-party project documents to this location. I believe the original file may pre-date that recedent. If we're updating this doc we should update the location as well. @clalancette @fujitatomoya do you have any thoughts with regard to this?

Yeah, I actually agree that we should move this to the "Related Projects" part of the site, because it is not part of the core. So @banisadr please do that as well.

@fujitatomoya fujitatomoya added the more-information-needed Further information is required label Oct 4, 2024
@fujitatomoya
Copy link
Collaborator

@banisadr friendly ping

@banisadr
Copy link
Contributor Author

banisadr commented Oct 4, 2024

Thanks @fujitatomoya @clalancette @kscottz! I moved it over to related projects and did a local test build. Nav/ToC seem to be working. Let me know if there's anything else I should update.

@clalancette clalancette removed the more-information-needed Further information is required label Oct 15, 2024
@banisadr
Copy link
Contributor Author

ping @fujitatomoya

@fujitatomoya
Copy link
Collaborator

@clalancette can you review this and merge?

.DS_Store Outdated Show resolved Hide resolved
@banisadr banisadr requested a review from ahcorde November 1, 2024 00:39
@banisadr
Copy link
Contributor Author

banisadr commented Nov 1, 2024

Ping @clalancette @audrow 🙏

@kscottz kscottz dismissed ahcorde’s stale review November 11, 2024 23:03

Looks like this was resolved in conversation. Let me know if this isn't the case.

@kscottz
Copy link
Collaborator

kscottz commented Nov 11, 2024

Looks like everything passes and all requests have been addressed. I'll go ahead and merge this. Thanks for being patient while we were all at ROSCon.

@kscottz kscottz merged commit bb41860 into ros2:rolling Nov 11, 2024
4 checks passed
@banisadr banisadr deleted the banisadr/foxglove-updates branch November 13, 2024 19:05
@banisadr
Copy link
Contributor Author

Thank you!

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.

7 participants