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

Add Dbow3 Library #8547

Merged
merged 2 commits into from
Oct 10, 2019
Merged

Add Dbow3 Library #8547

merged 2 commits into from
Oct 10, 2019

Conversation

RamadanAhmed
Copy link
Contributor

No description provided.

@MVoz
Copy link
Contributor

MVoz commented Oct 10, 2019

delete str
-DBUILD_SHARED_LIBS=ON

@vicroms
Copy link
Member

vicroms commented Oct 10, 2019

Thanks for your contribution!

@vicroms vicroms merged commit 065c9c4 into microsoft:master Oct 10, 2019
@cenit
Copy link
Contributor

cenit commented Oct 10, 2019

This is the original repository of this library... shall we change to that maybe??.......

https://github.com/rmsalinas/DBow3

@cenit
Copy link
Contributor

cenit commented Oct 10, 2019

Also I am not sure it really requires opencv3 and is not working with the generic opencv[contrib]

@RamadanAhmed
Copy link
Contributor Author

This is the original repository of this library... shall we change to that maybe??.......

https://github.com/rmsalinas/DBow3

I tried to build it on windows but it didn't go smoothly

Also I am not sure it really requires opencv3 and is not working with the generic opencv[contrib]

You are correct, but I was working with opencv3 so it asked me to rebuild the whole opencv package at the time

@cenit
Copy link
Contributor

cenit commented Oct 11, 2019

While I appreciate your efforts, usually it's best to link to the original repository and attach a patch to the portfile here to make it work. Personal repositories are not very well received by the downstream users (people that might want to use DBow3 but don't know you and so might assume it's a different library)

@RamadanAhmed
Copy link
Contributor Author

Thank you for the information
I will try to make a patch from diff in CMakelists.txt and make another PR soon

@MVoz
Copy link
Contributor

MVoz commented Oct 11, 2019

@RamadanAhmed
Copy link
Contributor Author

@voskrese Thanks alot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants