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

fix: sample projects, remove unused properties #2205

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

Jklawreszuk
Copy link
Collaborator

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

  • Refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

Copy link
Member

@Kryptos-FR Kryptos-FR left a 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.

@Jklawreszuk Jklawreszuk force-pushed the samples-fix branch 2 times, most recently from 68d4dee to b6411b0 Compare April 1, 2024 07:04
@Eideren Eideren requested a review from xen2 April 21, 2024 18:45
<ApplicationIcon>Resources\GameIcon.ico</ApplicationIcon>
<OutputType>WinExe</OutputType>
<RootNamespace>SimpleAudio</RootNamespace>
<OutputPath>..\Bin\Windows\$(Configuration)\</OutputPath>
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
<DefineConstants>STRIDE_PLATFORM_DESKTOP</DefineConstants>
Copy link
Member

@Kryptos-FR Kryptos-FR Aug 24, 2024

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.

Copy link
Member

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

@Eideren Eideren merged commit c424213 into stride3d:master Aug 28, 2024
2 checks passed
@Eideren
Copy link
Collaborator

Eideren commented Aug 28, 2024

Thanks for reviewing @Kryptos-FR and @Jklawreszuk for the PR of course

@Eideren Eideren changed the title Fix sample projects, remove unused properties fix: sample projects, remove unused properties Aug 28, 2024
@Jklawreszuk Jklawreszuk deleted the samples-fix branch August 28, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants