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

Code Quality: Migrated shared properties to props file #16557

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

Lamparter
Copy link
Contributor

@Lamparter Lamparter commented Dec 4, 2024

Resolved / Related Issues

Steps used to test these changes

Built the app; checked for Roslyn analyser warnings


This PR finally adds a props file to manage mutual properties where appropriate. It has been built and tested and works perfectly.

I also wanted to implement CPM however haven't succeeded yet.

@0x5bfa

This comment was marked as resolved.

I have absolutely no idea how to do that so hope this works
@Lamparter Lamparter changed the title Code Quality: Migrate shared properties to Directory.Build.props Code Quality: Migrated shared properties to Directory.Build.props Dec 7, 2024
@Lamparter Lamparter changed the title Code Quality: Migrated shared properties to Directory.Build.props Code Quality: Migrated shared properties to props file Dec 7, 2024
@Lamparter Lamparter marked this pull request as ready for review December 8, 2024 08:14
@0x5bfa
Copy link
Member

0x5bfa commented Dec 10, 2024

LGTM

@yaira2
Copy link
Member

yaira2 commented Dec 10, 2024

Can you please rebase from main?

Directory.Build.props Outdated Show resolved Hide resolved
Files.sln Outdated Show resolved Hide resolved
@Lamparter Lamparter requested a review from yaira2 December 12, 2024 07:38
@yaira2
Copy link
Member

yaira2 commented Dec 15, 2024

We're currently facing an issue where the x86 and ARM64 configurations are displaying an error for 'incorrect configurations'.

@yaira2 yaira2 added changes requested Changes are needed for this pull request and removed needs - code review labels Dec 15, 2024
@yaira2 yaira2 added needs - code review and removed changes requested Changes are needed for this pull request labels Dec 15, 2024
@0x5bfa
Copy link
Member

0x5bfa commented Dec 21, 2024

I think Files.sln got messed up but we should recreate a new .sln and add one by one. One more concern is Win32 configuration; we should have named it x86.

However, for now, as far as current solution file doesn't cause configuration issue, LGTM.

@Lamparter
Copy link
Contributor Author

Once this PR is merged, I'll create a completely new solution file in another PR and you can review.

@0x5bfa
Copy link
Member

0x5bfa commented Dec 21, 2024

I have a pr that simplify the configurations of the c++ projects, so let's wait it too.

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.

Code Quality: Introduce Directory.build.props for shared properties
4 participants