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

Emitter migration #1287

Merged
merged 9 commits into from
Mar 1, 2022
Merged

Emitter migration #1287

merged 9 commits into from
Mar 1, 2022

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Jan 12, 2022

🎉 Cleanup

Summary

During SubT we created a particle_emitter2 plugin designed to replace the particle_emitter plugin. This PR takes the next steps toward having just one particle emitter system.

I tried to make the series of commits easy to follow:

  1. 038b59a: Removed the particle_emitter directory
  2. 84df817: Copied particle_emitter2 to particle_emitter.
  3. 4e39df2: Removed particle_emitter test files and example world.
  4. c499516: Moved particle_emitter2 test files and example world to particle_emitter.
  5. cc67651: Renamed emitter2 to emitter inside the test files and example world.
  6. 3038fe0: Fixed the integration test.
  7. deaf1b1: Updated the tutorial and lrauv world.

Test it

Checklist

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.

@github-actions github-actions bot added the 🌱 garden Ignition Garden label Jan 12, 2022
@codecov
Copy link

codecov bot commented Jan 12, 2022

Codecov Report

Merging #1287 (d096437) into main (65efda9) will decrease coverage by 0.01%.
The diff coverage is 51.56%.

❗ Current head d096437 differs from pull request most recent head abbb007. Consider uploading reports for the commit abbb007 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1287      +/-   ##
==========================================
- Coverage   62.32%   62.31%   -0.02%     
==========================================
  Files         278      277       -1     
  Lines       23242    23102     -140     
==========================================
- Hits        14486    14396      -90     
+ Misses       8756     8706      -50     
Impacted Files Coverage Δ
src/systems/particle_emitter/ParticleEmitter.cc 55.40% <51.56%> (-10.31%) ⬇️
src/Server.cc 86.14% <0.00%> (-0.61%) ⬇️
src/systems/particle_emitter2/ParticleEmitter2.cc
src/Conversions.cc 82.88% <0.00%> (+0.32%) ⬆️

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 faaa3ec...abbb007. Read the comment docs.

@chapulina chapulina added the rendering Involves Ignition Rendering label Jan 13, 2022
Copy link
Contributor

@iche033 iche033 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 to me. Add an entry to the migration guide?

Nate Koenig added 2 commits February 14, 2022 09:10
@nkoenig
Copy link
Contributor Author

nkoenig commented Feb 14, 2022

Migration guide updated in 9f7648a

1 similar comment
@nkoenig
Copy link
Contributor Author

nkoenig commented Feb 14, 2022

Migration guide updated in 9f7648a

@iche033
Copy link
Contributor

iche033 commented Feb 14, 2022

hmm something is not quite right with the ubuntu build. The test failures don't look related to changes in this PR. I just triggered another build to double check.

@iche033
Copy link
Contributor

iche033 commented Feb 28, 2022

@osrf-jenkins run tests please

Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

ubuntu build is better now. Don't think that the test failures are related.

@nkoenig nkoenig merged commit c58a968 into main Mar 1, 2022
@nkoenig nkoenig deleted the emitter_migration branch March 1, 2022 18:47
@j-rivero j-rivero mentioned this pull request Sep 16, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden rendering Involves Ignition Rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants