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

Nkoenig/3 to 4 20210113 #554

Merged
merged 5 commits into from
Jan 13, 2021
Merged

Nkoenig/3 to 4 20210113 #554

merged 5 commits into from
Jan 13, 2021

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Jan 13, 2021

Merge 3 to 4.

mjcarroll and others added 4 commits January 8, 2021 08:28
Feature to allow loading of a default set of system plugins from a file. This behavior will trigger when a world sdf file is loaded with no plugins defined.  In this case, the simulator will load the plugins from a series of locations including environment variable, the users home folder, and finally in the installation directory.

This should allow users to not have to specify the same set of plugins in every world sdf file.

Signed-off-by: Michael Carroll <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
* Added missing version namespace

Signed-off-by: Nate Koenig <[email protected]>

* Fix codecheck

Signed-off-by: Nate Koenig <[email protected]>

Co-authored-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>

Co-authored-by: Nate Koenig <[email protected]>
@nkoenig nkoenig requested a review from chapulina as a code owner January 13, 2021 17:48
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Jan 13, 2021
@chapulina
Copy link
Contributor

I'm not seeing the changes from #543 on the diff, not sure what happened

@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #554 (94c6902) into ign-gazebo4 (e9e9f19) will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo4     #554      +/-   ##
===============================================
+ Coverage        77.25%   77.30%   +0.05%     
===============================================
  Files              213      213              
  Lines            11913    11913              
===============================================
+ Hits              9203     9209       +6     
+ Misses            2710     2704       -6     
Impacted Files Coverage Δ
...e/ignition/gazebo/detail/EntityComponentManager.hh 95.00% <ø> (ø)
src/SimulationRunner.cc 94.04% <0.00%> (+1.11%) ⬆️

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 e9e9f19...94c6902. Read the comment docs.

@nkoenig
Copy link
Contributor Author

nkoenig commented Jan 13, 2021

I'm not seeing the changes from #543 on the diff, not sure what happened

I'm not sure either. The updates are in place, but I don't know how they got there. See https://github.com/ignitionrobotics/ign-gazebo/blob/ign-gazebo4/tutorials/migration_plugins.md.

@chapulina
Copy link
Contributor

The updates are in place

I don't see the updates 🧐 Note that the first change was independently done to that branch. But all others, like this->connection, aren't fixed yet.

Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina force-pushed the nkoenig/3-to-4-20210113 branch from 20beaee to 94c6902 Compare January 13, 2021 20:01
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

I redid the merge and force-pushed to this branch and now I can see the diff.

LGTM with happy CI.

@nkoenig nkoenig merged commit 6fcbcea into ign-gazebo4 Jan 13, 2021
@nkoenig nkoenig deleted the nkoenig/3-to-4-20210113 branch January 13, 2021 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants