-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Optimise conditional compiles & package dependencies #1346
base: main
Are you sure you want to change the base?
Optimise conditional compiles & package dependencies #1346
Conversation
Thanks for the big PR! Can you also approve the Contributor License Agreement so we can look at merging this, thanks. |
We don't want to mix whitespace changes with other code changes in the same PR which makes it a huge PR for us to inititally review and and for others later when looking through the code base commit history. If you can remove the whitespace changes we can look at accepting the build symbol changes separately. For widespread whitespace changes like this we'll look at running a tool like tool like dotnet format ideally from a GitHub action. |
0f1ceb7
to
9fe000e
Compare
Currently working through painfully removing white space changes. |
7b714f2
to
34c752d
Compare
Wouldn't it be easier to stash your symbol changes, then reset back your codebase back to head then reapply your symbol changes? This PR is currently in conflict, not sure it's going to be mergeable after your changes. IMO its better to start with a clean PR. |
b0b0222
to
ad470a3
Compare
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.
Whitespace changes should now all be reverted and merge conflict was easily fixed. 😃
2075487
to
647c24a
Compare
Gone through and cleaned up conditional compilations to reduce future maintenance and removed redundant conditional code.
At the same time some packages have become conditional when they can be provided by the framework.