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

Changes ignition for gz in tutorial commands #570

Merged
merged 4 commits into from
Sep 14, 2023

Conversation

Voldivh
Copy link
Contributor

@Voldivh Voldivh commented Sep 14, 2023

🦟 Bug fix

Fixes gazebosim/gazebo_test_cases#564

Summary

Changes the use of ignition for gz in tutorial commands.

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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.

@Voldivh Voldivh changed the base branch from gz-gui7 to gz-gui8 September 14, 2023 17:45
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Sep 14, 2023
@Voldivh Voldivh changed the title Voldivh/changes ignition for gz Changes ignition for gz in tutorial commands Sep 14, 2023
@@ -98,14 +98,14 @@ Build and install as follows:

1. Clone the repository
```
git clone https:/gazebosim/gz-gui -b ign-gui<#>
git clone https:/gazebosim/gz-gui -b gz-gui<#>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a note that if the version is > gui7 then to use ign-? It also might be good to add a link to this post: https://community.gazebosim.org/t/a-new-era-for-gazebo/1356

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Could you take a look?

Copy link
Contributor

@jennuine jennuine Sep 14, 2023

Choose a reason for hiding this comment

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

Statement looks good, do you mind putting it closer to the beginning of this document, right above Binary Install heading here:

## Binary Install

I'd make sure to add a empty row before and afterwards so the Note is on it's own line, rather than part of the previous sentence.

@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Merging #570 (5a5a54c) into gz-gui8 (20616c1) will not change coverage.
The diff coverage is n/a.

❗ Current head 5a5a54c differs from pull request most recent head 15df2c9. Consider uploading reports for the commit 15df2c9 to get more accurate results

@@           Coverage Diff            @@
##           gz-gui8     #570   +/-   ##
========================================
  Coverage    68.10%   68.10%           
========================================
  Files           38       38           
  Lines         5367     5367           
========================================
  Hits          3655     3655           
  Misses        1712     1712           

Copy link
Contributor

@jennuine jennuine left a comment

Choose a reason for hiding this comment

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

Looks good, one small nit :)

@@ -98,14 +98,14 @@ Build and install as follows:

1. Clone the repository
```
git clone https:/gazebosim/gz-gui -b ign-gui<#>
git clone https:/gazebosim/gz-gui -b gz-gui<#>
Copy link
Contributor

@jennuine jennuine Sep 14, 2023

Choose a reason for hiding this comment

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

Statement looks good, do you mind putting it closer to the beginning of this document, right above Binary Install heading here:

## Binary Install

I'd make sure to add a empty row before and afterwards so the Note is on it's own line, rather than part of the previous sentence.

@jennuine
Copy link
Contributor

Can you sign commits please: https:/gazebosim/gz-gui/pull/570/checks?check_run_id=16804603853

For future, when making commits, you can do git commit -sm "commit message" to add the signoff to your commits.

@Voldivh
Copy link
Contributor Author

Voldivh commented Sep 14, 2023

Can you sign commits please: https:/gazebosim/gz-gui/pull/570/checks?check_run_id=16804603853

For future, when making commits, you can do git commit -sm "commit message" to add the signoff to your commits.

Sure thing. I'm not sure why this is happening, I usually just do git commit -s -m "commit message"

@Voldivh Voldivh force-pushed the voldivh/changes_ignition_for_gz branch from 5a5a54c to 15df2c9 Compare September 14, 2023 19:44
@jennuine
Copy link
Contributor

I'm not sure why this is happening, I usually just do git commit -s -m "commit message"

That works too. I believe your first and last commit were signed but the two middle ones weren't. If you like, there is a way to set this up automatically so you don't always have to remember -s: https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits

@Voldivh
Copy link
Contributor Author

Voldivh commented Sep 14, 2023

I'm not sure why this is happening, I usually just do git commit -s -m "commit message"

That works too. I believe your first and last commit were signed but the two middle ones weren't. If you like, there is a way to set this up automatically so you don't always have to remember -s: https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits

Thanks Jenn, I'll take a look into it. Could have easily missed a couple by accident :D

@jennuine jennuine added bug Something isn't working 🎵 harmonic Gazebo Harmonic and removed 🌱 garden Ignition Garden labels Sep 14, 2023
@jennuine jennuine merged commit e115d56 into gz-gui8 Sep 14, 2023
6 of 9 checks passed
@jennuine jennuine deleted the voldivh/changes_ignition_for_gz branch September 14, 2023 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

gz-gui: Installation
2 participants