-
-
Notifications
You must be signed in to change notification settings - Fork 619
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
Fix visibility setting on android and gcc VS #2335
base: master
Are you sure you want to change the base?
Conversation
Commit d363e35 broke visibility by not updating internal code to use new location
Can you add a unit test that demonstrates the bug (and by proxy, the fix)? |
Yes, I guess I overlooked it. My mistake. |
@nickclark2016 As stated I wasn't sure which test file to put it in. Please tell me where to put it 🙂 |
For the VS module - |
Hi, just wanted to poke on this to see if any progress has been made on UTs |
@alex-rass-88 Just wanted to poke on this again, before we freeze the master branch for beta5's release this coming weekend. |
Now change need in one file: premake-core/modules/vstudio/vs2010_vcxproj.lua Line 4448 in 801e6a0
table.insert(opts, p.tools.gcc.cxxflags.visibility[cfg.visibility]) ==> table.insert(opts, p.tools.gcc.shared.visibility[cfg.visibility]) |
For tests need add to modules/vstudio/tests/android/test_android_project.lua:
|
Commit d363e35 broke visibility by not updating internal code to use new location
What does this PR do?
Fixes the
visibility
setting for android and gcc VS projects (untested for gcc VS, just noticed it used the same old location forvisibility
)How does this PR change Premake's behavior?
Doesn't error anymore
Anything else we should know?
Wanted to add a unit test but no idea in which file to add it
Did you check all the boxes?
closes #XXXX
in comment to auto-close issue when PR is merged)