-
-
Notifications
You must be signed in to change notification settings - Fork 909
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 sample projects, remove unused properties #2205
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comments to a single file, but it applies to all.
samples/Audio/SimpleAudio/SimpleAudio.Windows/SimpleAudio.Windows.csproj
Outdated
Show resolved
Hide resolved
<DebugSymbols>true</DebugSymbols> | ||
<DebugType>full</DebugType> | ||
<Optimize>false</Optimize> | ||
<DefineConstants>DEBUG;TRACE;STRIDE_PLATFORM_WINDOWS;STRIDE_PLATFORM_WINDOWS_DESKTOP</DefineConstants> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure the Stride constants don't need to be set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that these constants do not exists anywhere in source code except in props file here https://github.com/stride3d/stride/blob/master/sources/targets/Stride.Core.props#L44, but in the end it has no affect on the app behavior at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xen2 are they really unused, or could they be used by TeamCity tests for categorization?
They are not set on projects created by users, so the only candidate I can think of would be for testing. If not, let's remove them indeed.
PR Details
Description
Current samples are using incorrect net8 target framework moniker (windows 7 is no longer supported). I also decided to remove additional bloat from project files
Types of changes
Checklist