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

Bring in support for mermaid and move pinning forward #4151

Merged
merged 6 commits into from Feb 5, 2024
Merged

Conversation

tfoote
Copy link
Contributor

@tfoote tfoote commented Jan 30, 2024

I'm working with @gonzodepedro to bring in sequence diagrams to explain the idl generation. And we got bogged down in trying to get mermaid added to the system.

Without being able to do this we're working on committing the mmd files as well as the images.

We had pinned back everything to match Jammy. I think that pinning everything makes sense, but I think that we should allow rolling forward before Ubuntu does, but we should stick to virtualenv supported versions to not get too far out of date. To that end this is validating and pulling the pins all forward to a pip upgrade. It works for both the local build and the multiversion build on my local machine.

For reference we're working on documentation improvements like this: https://github.com/gonzodepedro/ros2_documentation/pull/1/files#diff-4cfad9479ac6c8d7db3ebcaaa176863bcf74f19ebbb2a101c281d6439e801c12

To get mermaide support working I had to do upgrades. I've updated our pins and documented how they're used as well as how to update them too.

This will allow us to do integrated mermaid sequence diagrams as well as flow diagrams instead of committing images of them.
@tfoote tfoote changed the title Bring in support for mermaid Bring in support for mermaid and move pinning forward Jan 30, 2024
@tfoote tfoote added the backport-all backport at reviewers discretion; from rolling to all versions label Jan 30, 2024
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

I tested with Jammy make html and make multiversion they both did work w/o any warnings.

.gitignore Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tfoote and others added 2 commits January 31, 2024 08:19
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Tully Foote <tully.foote@gmail.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Tully Foote <tully.foote@gmail.com>
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm. i would like someone else to review this before merge.

.gitignore Outdated
@@ -3,3 +3,4 @@ plugins/__pycache__
_build/
.idea/
.vscode/
__pycache__/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I think we can remove line 2 since this ignores __pychache__ folders at any level?

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

A couple of nits.

README.md Outdated Show resolved Hide resolved
@kscottz
Copy link
Collaborator

kscottz commented Jan 31, 2024

When this lands can one of you ping me? I think we should make an announcement so the broader community knows about it.

Signed-off-by: Tully Foote <tullyfoote@intrinsic.ai>
Signed-off-by: Tully Foote <tullyfoote@intrinsic.ai>
@tfoote tfoote merged commit c569b50 into rolling Feb 5, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the try_mermaid branch February 5, 2024 16:08
mergify bot pushed a commit that referenced this pull request Feb 5, 2024
* Add mermaid support and update pinned version

To get mermaid support working I had to do upgrades. I've updated our pins and documented how they're used as well as how to update them too.

This will allow us to do integrated mermaid sequence diagrams as well as flow diagrams instead of committing images of them.

* clean up docker installation to focus on pip version pinning

* Match folders in ignore too

Signed-off-by: Tully Foote <tullyfoote@intrinsic.ai>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit c569b50)
mergify bot pushed a commit that referenced this pull request Feb 5, 2024
* Add mermaid support and update pinned version

To get mermaid support working I had to do upgrades. I've updated our pins and documented how they're used as well as how to update them too.

This will allow us to do integrated mermaid sequence diagrams as well as flow diagrams instead of committing images of them.

* clean up docker installation to focus on pip version pinning

* Match folders in ignore too

Signed-off-by: Tully Foote <tullyfoote@intrinsic.ai>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit c569b50)
tfoote added a commit that referenced this pull request Feb 5, 2024
* Add mermaid support and update pinned version

To get mermaid support working I had to do upgrades. I've updated our pins and documented how they're used as well as how to update them too.

This will allow us to do integrated mermaid sequence diagrams as well as flow diagrams instead of committing images of them.

* clean up docker installation to focus on pip version pinning

* Match folders in ignore too

Signed-off-by: Tully Foote <tullyfoote@intrinsic.ai>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit c569b50)

Co-authored-by: Tully Foote <tullyfoote@intrinsic.ai>
tfoote added a commit that referenced this pull request Feb 5, 2024
* Add mermaid support and update pinned version

To get mermaid support working I had to do upgrades. I've updated our pins and documented how they're used as well as how to update them too.

This will allow us to do integrated mermaid sequence diagrams as well as flow diagrams instead of committing images of them.

* clean up docker installation to focus on pip version pinning

* Match folders in ignore too

Signed-off-by: Tully Foote <tullyfoote@intrinsic.ai>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit c569b50)

Co-authored-by: Tully Foote <tullyfoote@intrinsic.ai>
@kscottz
Copy link
Collaborator

kscottz commented Feb 5, 2024

Quick questions:

  • Do we need to add any installation steps to the root readme for the docs?
  • Did we land an example that I can point to? Is there perhaps something in flight?

@tfoote
Copy link
Contributor Author

tfoote commented Feb 5, 2024

@kscottz

The needed extras are in the requirements.txt so if you follow the installation instructions you get it. Existing installations will need to rerun the installer.

An example using it: gonzodepedro@d580364#diff-c9bf592e9b9e4321631795943475e03abda9bb1bd2634b831482f53355f370dcR232

It will be rebased and open as PR on top of this shortly, but it's not landed yet as this was pending.

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-news-for-the-week-of-february-12th-2024/36135/1

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/discuss-tools-and-best-practices-for-3d-robot-assets-urdf-sdformat-cad-etc/36997/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-all backport at reviewers discretion; from rolling to all versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants