Skip to content

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.

3 participants