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

Don't export vorbisenc functions from vorbis.dll with CMake #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

evpobr
Copy link
Contributor

@evpobr evpobr commented Apr 10, 2020

No description provided.

Copy link
Contributor

@rillian rillian left a comment

Choose a reason for hiding this comment

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

I don't like the duplication of the .def file. If our goal is to replace the visual studio project files with cmake, and you want to change the ABI, we should just change the ABI for all win32 builds.

@rillian
Copy link
Contributor

rillian commented Apr 15, 2020

Please also add some documentation about why the export list changed to the commit message, so anyone looking for the change has an easier time understanding the motivation.

@evpobr
Copy link
Contributor Author

evpobr commented Apr 21, 2020

I don't like the duplication of the .def file. If our goal is to replace the visual studio project files with cmake, and you want to change the ABI, we should just change the ABI for all win32 builds.

I don't understand. You want from me also fixing Visual Studio solutions to use updated DEF file?

@rillian
Copy link
Contributor

rillian commented Apr 30, 2020

That's correct. If we're changing the vorbis.dll ABI I think we should change it for all the builds.

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.

2 participants