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

Added missing version namespace #541

Merged
merged 2 commits into from
Jan 11, 2021
Merged

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Jan 8, 2021

Adding this to help fix downstream users of ign-gazebo, such as ign-launch.

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

@nkoenig nkoenig requested a review from chapulina as a code owner January 8, 2021 23:12
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Jan 8, 2021
Signed-off-by: Nate Koenig <[email protected]>
@codecov
Copy link

codecov bot commented Jan 8, 2021

Codecov Report

Merging #541 (f75efff) into ign-gazebo3 (6b6f76f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           ign-gazebo3     #541   +/-   ##
============================================
  Coverage        77.61%   77.61%           
============================================
  Files              208      208           
  Lines            11526    11526           
============================================
  Hits              8946     8946           
  Misses            2580     2580           
Impacted Files Coverage Δ
...e/ignition/gazebo/detail/EntityComponentManager.hh 95.00% <ø> (ø)

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 6b6f76f...f75efff. Read the comment docs.

@nkoenig nkoenig merged commit 69e190c into ign-gazebo3 Jan 11, 2021
@nkoenig nkoenig deleted the missing_version_namespace branch January 11, 2021 19:58
@j-rivero
Copy link
Contributor

That one was an interesting change. inline namespaces are specially designed to modify the binary symbols names so changing it in a released branch should ring the alarms. However, the abichecker is happy with it. In fact looking for some code modified here does not appear anywhere in binaries:

/usr/lib/x86_64-linux-gnu🔒 ❯ nm -D --demangle  libignition-gazebo3-* | grep EntityComponentManager | grep First
/usr/lib/x86_64-linux-gnu🔒 ❯ 

My hypothesis is that the file belongs to a private part (details) and that makes the real work of being private and not exposed to public ABI. I might be wrong and the abichecker could also be wrong and weird problems might appear somehow in the new release. Keep an eye on these change if that happen.

nkoenig added a commit that referenced this pull request Jan 13, 2021
* Automatically load a subset of world plugins (#281)

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 (#541)

* 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]>

* Fix examples in migration plugins tutorial (#543)

Signed-off-by: Nick Lamprianidis <[email protected]>

* Prepare for 3.7.0 release (#552)

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

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

Co-authored-by: Michael Carroll <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>
Co-authored-by: Nick Lamprianidis <[email protected]>
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.

3 participants