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

Fix updating GUI plugin on load #904

Merged
merged 1 commit into from
Jul 15, 2021
Merged

Conversation

chapulina
Copy link
Contributor

🦟 Bug fix

Summary

Ignition GUI has been sending an empty string until now, see gazebosim/gz-gui#249. With that fix, the signal will send a unique string instead of the plugin name.

The current code is always getting the first plugin loaded, because they all have empty names. With this fix, the newly added plugin should be correctly got.

If the ign-gui PR is released without this fix, users will be spammed with error messages that they can't get the plugin with name plugin<number>.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina requested a review from ahcorde July 7, 2021 01:18
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Jul 7, 2021
@ahcorde ahcorde added the needs upstream release Blocked by a release of an upstream library label Jul 7, 2021
@ahcorde
Copy link
Contributor

ahcorde commented Jul 14, 2021

@osrf-jenkins retest this please

@chapulina chapulina mentioned this pull request Jul 14, 2021
7 tasks
@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #904 (4fb0c0d) into ign-gazebo3 (15b456b) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 4fb0c0d differs from pull request most recent head 1291440. Consider uploading reports for the commit 1291440 to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo3     #904      +/-   ##
===============================================
- Coverage        77.94%   77.92%   -0.02%     
===============================================
  Files              215      215              
  Lines            12077    12080       +3     
===============================================
  Hits              9413     9413              
- Misses            2664     2667       +3     
Impacted Files Coverage Δ
src/gui/GuiRunner.cc 93.75% <100.00%> (-4.62%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15b456b...1291440. Read the comment docs.

@ahcorde
Copy link
Contributor

ahcorde commented Jul 15, 2021

@osrf-jenkins retest this please

@ahcorde ahcorde enabled auto-merge (squash) July 15, 2021 08:11
@ahcorde ahcorde merged commit 65c7e9a into ign-gazebo3 Jul 15, 2021
@ahcorde ahcorde deleted the chapulina/3/plugin_name branch July 15, 2021 08:47
@ahcorde ahcorde self-assigned this Jul 15, 2021
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Jul 15, 2021
@adlarkin adlarkin mentioned this pull request Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants