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 net40 client profile #66

Merged
merged 1 commit into from Sep 17, 2020
Merged

Conversation

NicholasNoise
Copy link

log4net_net40cp

@NicholasNoise
Copy link
Author

@fluffynuts

@cremor
Copy link

cremor commented Sep 8, 2020

Not directly related to this PR, but since you are modifying the preprocessor symbols (constants) here I thought I should mention this:
SDK style projects already define various target framework related constants automatically, see https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/preprocessor-directives/preprocessor-if#remarks

Some of those are the same as are now manually defined in the log4net.csproj and some are different. This might be confusing for developers who don't check if the constants are defined by the project itself.

But it also seems like migration to the official constants is not that easy. Because with them it seems like the version specific constants are not enabled for lower versions when a higher version is built.

@NicholasNoise
Copy link
Author

@cremor

Some of those are the same as are now manually defined in the log4net.csproj and some are different. This might be confusing for developers who don't check if the constants are defined by the project itself.

But it also seems like migration to the official constants is not that easy. Because with them it seems like the version specific constants are not enabled for lower versions when a higher version is built.

You're right, no other way about it. Build-in features are preferable.
But it seems unnecessary since this project is dormant.

I've spent almost two working day to enable netstandard2.0, so yes, is not easy... until old targets are supported.
Should be much easier without legacy preprocessor symbols like 'NETCF', 'CLIENT_PROFILE' and even 'NET20'

@fluffynuts
Copy link
Contributor

LGTM; when I have a moment, I'd like to pull down to my machine & build again to double-check; I'm taking a day off from work tomorrow, so perhaps Thursday (:

@NicholasNoise
Copy link
Author

@fluffynuts last Thursday or this one? =)

@fluffynuts
Copy link
Contributor

Sorry, been a bit mad -- the day off turned into a mission (:

anyway, LGTM, merging.

@fluffynuts fluffynuts merged commit 3fa6542 into apache:master Sep 17, 2020
@fluffynuts fluffynuts deleted the fix-client-profile branch September 17, 2020 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants