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

Remove lib+.so from plugin's name #279

Merged
merged 10 commits into from
Sep 1, 2020

Conversation

mohamedsayed18
Copy link
Contributor

Fix #277.
Is this PR big and need to be broken down into smaller ones?

Signed-off-by: mohamedsayed18 <[email protected]>
Signed-off-by: mohamedsayed18 <[email protected]>
Signed-off-by: mohamedsayed18 <[email protected]>
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

It doesn't look like you consistently remove the .so extension from the plugin filenames. Is there a reason for this?

@mohamedsayed18
Copy link
Contributor Author

mohamedsayed18 commented Aug 12, 2020

It doesn't look like you consistently remove the .so extension from the plugin filenames. Is there a reason for this?

No, it was just a mistake.
I searched for the .so and I found these two files:

  • ./build/ignition-gazebo3/CMakeCache.txt:325 //Install .so files without execute permission.
  • tutorials/physics.md:97 If that's a 3rd party engine, find where the .so file is installed and add

Should I remove those too?

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.

Is this PR big and need to be broken down into smaller ones?

I think that's fine because it's so repetitive

Should I remove those too?

No, those can stay

examples/scripts/distributed/primary.sdf Outdated Show resolved Hide resolved
@mohamedsayed18
Copy link
Contributor Author

I think I shouldn't remove it from the custom plugins like libCustomEngine.so, libLiftDragPlugin.so, libSampleSystem.so?

Should I remove it from the ign-gazebo/test directory?

Signed-off-by: mohamedsayed18 <[email protected]>
@chapulina
Copy link
Contributor

I think I shouldn't remove it from the custom plugins

It can be removed from all of then, ign-plugin will be able to find them 😉

Should I remove it from the ign-gazebo/test directory?

That would be great, thanks!

@mohamedsayed18
Copy link
Contributor Author

I will work on it.

Signed-off-by: mohamedsayed18 <[email protected]>
Signed-off-by: mohamedsayed18 <[email protected]>
Signed-off-by: mohamedsayed18 <[email protected]>
@mohamedsayed18
Copy link
Contributor Author

I didn't remove the extension name from the src/ and the test/integration

@chapulina chapulina merged commit 4ebe95c into gazebosim:ign-gazebo3 Sep 1, 2020
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