-
-
Notifications
You must be signed in to change notification settings - Fork 964
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
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
samples/Audio/SimpleAudio/SimpleAudio.Windows/SimpleAudio.Windows.csproj
Show resolved
Hide resolved
samples/Audio/SimpleAudio/SimpleAudio.Windows/SimpleAudio.Windows.csproj
Show resolved
Hide resolved
68d4dee
to
b6411b0
Compare
<ApplicationIcon>Resources\GameIcon.ico</ApplicationIcon> | ||
<OutputType>WinExe</OutputType> | ||
<RootNamespace>SimpleAudio</RootNamespace> | ||
<OutputPath>..\Bin\Windows\$(Configuration)\</OutputPath> | ||
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath> | ||
<DefineConstants>STRIDE_PLATFORM_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.
It looks like the TRACE
constants was also defined in all configurations. Not sure what it is used for though.
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 to be useful when we want to use the Trace class and/or print out trace logs. I don't think it is a useful default for game samples. So I'm ok with leaving it out.
Reference: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.trace
Thanks for reviewing @Kryptos-FR and @Jklawreszuk for the PR of course |
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