Skip to content

Added documentation for usage of setup-gazebo #530

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

Merged
merged 5 commits into from
Oct 29, 2024

Conversation

sauk2
Copy link
Contributor

@sauk2 sauk2 commented Oct 20, 2024

🎉 New feature

Closes #529

Summary

Added a new section with information on setup-gazebo, followed by subsections on how to use it on each of the three supported platforms.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@sauk2 sauk2 requested a review from mabelzhang as a code owner October 20, 2024 12:07
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for adding this documentation. I only have one small comment.


* Citadel
* Fortress
* Garden
Copy link
Contributor

Choose a reason for hiding this comment

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

Garden is pretty much EOL now, so it can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing it out.

However, currently, the action reads from the collections file - https://github.com/gazebo-tooling/release-tools/blob/8483b5c73e47aa1e098f7dbbee29a7b3fc0ae3b8/jenkins-scripts/dsl/gz-collections.yaml#L190

So it will still support the installation of Garden as long as it is in that file. Would you still like me to remove it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we can remove it yes. I will remove it from collections as soon as the release is EOL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the clarification. I have removed the mention of Garden.

@j-rivero
Copy link
Contributor

Thanks @sauk2 for the PR. I think that there is a good part of the content directly copied from the README.md in the setup-gazebo repository. I'm worried about the amount of maintenance work that this can cause when we update/change the setup-gazebo README. I would be more inclined to create a single document where we point the users to read the README file in github or the Marketplace instructions (the same text) of the setup-gazebo instead of recreating the content here. We can have a nice introduction about CI like the one we have now.

@sauk2
Copy link
Contributor Author

sauk2 commented Oct 28, 2024

Thanks @sauk2 for the PR. I think that there is a good part of the content directly copied from the README.md in the setup-gazebo repository. I'm worried about the amount of maintenance work that this can cause when we update/change the setup-gazebo README. I would be more inclined to create a single document where we point the users to read the README file in github or the Marketplace instructions (the same text) of the setup-gazebo instead of recreating the content here. We can have a nice introduction about CI like the one we have now.

Thanks for the feedback! This makes a lot of sense. I have updated instructions to direct users to the Marketplace page.

Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

yay !! I really like it. Good to go if @azeey is fine with it. Thanks so much @sauk2 !!

@azeey azeey enabled auto-merge (squash) October 29, 2024 16:01
@azeey azeey merged commit 2006f68 into gazebosim:master Oct 29, 2024
5 checks passed
@sauk2 sauk2 deleted the setup-gazebo-docs branch October 29, 2024 16:24
azeey added a commit to azeey/docs that referenced this pull request Jan 9, 2025
* Added documentation for usage of setup-gazebo

Signed-off-by: Saurabh Kamat <[email protected]>

* Removed garden from setup-gazebo docs

Signed-off-by: Saurabh Kamat <[email protected]>

* Updated setup-gazebo usage instructions

Signed-off-by: Saurabh Kamat <[email protected]>

---------

Signed-off-by: Saurabh Kamat <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add documentation for usage of setup-gazebo
3 participants