You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2020/09/02 06:12:38 UTC
[GitHub] [logging-log4net] NicholasNoise opened a new pull request #63: Implimented proper support of netstandard2.0
NicholasNoise opened a new pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63
Changelog:
- removed nuspec using build-in csproj packing.
- miltiframework test runner: net462, netcoreapp1.0, netcoreapp2.1, netcoreapp3.1 (for future builds of netstandard2.1 or netcoreapp)
- split NETSTANDARD1_3 and NETSTANDARD2_0 constant (replaced with NETSTANDARD)
- netstandard2.0 now supports configuration, adonet and other features.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-686943609
@NicholasNoise please have a look over the changes I've made to see that you're happy with them.
Also, since I know building for the older targets can be a mission, I've added a helper script to get dotnet core 1.1 & updated the docs to draw attention to them. Please try the `install-net-framework-sdk-3.5.ps1` helper to see if that resolves your issue with not being able to build against Client Profile (net35 and net40). I last got my machine building against these targets some time ago, so it would help to have verification that the script I wrote then still works (:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685695290
@fluffynuts Thank you for quick response!
I made this PR from non-personal fork, but already requested branch setting change to an admin of organization (should be granted by now)
1. I am not familiar with definition of log4net's build system and hardly understand how it really works. So better you than me :)
By default VS2019 doesnt support netcoreapp1.0, but sdk can be downloaded and installed manually ([link](https://dotnet.microsoft.com/download/dotnet-core/1.0))
Also I've dropped `net35-client` and `net40-client` as target frameworks, because unable to compile against. Should we bring them back and test somehow or deprecate and drop support in 2020?
2. `nupgk` output directory maybe customized by `PackageOutputPath` specified in csproj:
```
<PackageOutputPath>..\..\build\artifacts</PackageOutputPath>
```
I should mention that csproj has no support of some nuspec properties: `summary` was deprecated, `owners` was also deprecated and should be specified on nuget.org package page.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691513310
@NicholasNoise the vote passed, package 2.0.10 is up at nuget.org; the site should hopefully be updated when someone with the required permissions to publish has some time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-687882632
I've started the vote process, so it's pretty-much up to the rest of the list and how much I've managed to mess up on this release (it's only my second, but the first could have been way better, so :crossed_fingers: )
I've also included a fix for a long-standing CVE, so I'm hoping that will make some other people happy too (:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691525273
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691513310
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691513310
@NicholasNoise the vote passed, package 2.0.10 is up at nuget.org; the site should hopefully be updated when someone with the required permissions to publish has some time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685935299
> Reflecting my previous experience adding or dropping targets are rarely considered as breaking\major change, minor at best.
it's breaking for people who depend on it; I'd (personally) like to drop client-profile targets, but I'm not ready to yet and I think that doing so would justify a major version increment
> As a reminder to @fluffynuts: better check content of nupgk before publishing it.
I did, but I forgot to check on client-profile outputs because I'm a derp. Now that I'm reminded of it, I remember _not_ seeing those outputs and I'll add them back in again.
Yes, it's a PITA to get the environment required to build all targets (I've not only done it more than once, but struggled to find off-premise CI to do so!) -- but AppVeyor can do it, so I guess perhaps the real solution might be to provide a non-CP .csproj to encourage contributions, at least for now, or to accept that (again, at least for now), this is a sticky situation.
It doesn't help that getting the SDK for these environments is such a mission because they are well beyond their support lifetime; again, I'd _like_ to drop support for unsupported frameworks (and that includes netapp1.(0|1)!), but that should come in a slightly less shocking format, I think.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691525273
@fluffynuts thanks!
I'll test it in live environment and come back with a feedback.
btw, this release is a bit messed up ([1](https://github.com/apache/logging-log4net/blob/master/src/log4net/AssemblyInfo.cs#L57), [2](https://github.com/apache/logging-log4net/blob/master/package.json#L3), #66), but harm done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685935299
> Reflecting my previous experience adding or dropping targets are rarely considered as breaking\major change, minor at best.
it's breaking for people who depend on it; I'd (personally) like to drop client-profile targets, but I'm not ready to yet and I think that doing so would justify a major version increment
> As a reminder to @fluffynuts: better check content of nupgk before publishing it.
I did, but I forgot to check on client-profile outputs because I'm a derp. Now that I'm reminded of it, I remember _not_ seeing those outputs and I'll add them back in again.
Yes, it's a PITA to get the environment required to build all targets (I've not only done it more than once, but struggled to find off-premise CI to do so!) -- but AppVeyor can do it, so I guess perhaps the real solution might be to provide a non-CP .csproj to encourage contributions, at least for now, or to accept that (again, at least for now), this is a sticky situation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691525273
@fluffynuts thanks!
I'll test it in live environment and come back with a feedback.
btw, this release is a bit messed up ([1](https://github.com/apache/logging-log4net/blob/master/src/log4net/AssemblyInfo.cs#L57), [2](https://github.com/apache/logging-log4net/blob/master/package.json#L3), #66), but no harm done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691513310
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691513310
@NicholasNoise the vote passed, package 2.0.10 is up at nuget.org; the site should hopefully be updated when someone with the required permissions to publish has some time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] cremor commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
cremor commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685717421
> Also I've dropped `net35-client` and `net40-client` as target frameworks, because unable to compile against. Should we bring them back and test somehow or deprecate and drop support (someone uses client profile this year?)?
I'd be very careful about dropping target frameworks. This is a breaking change. All projects using that framework will no longer be able to use the newer versions of log4net. I don't know if log4net follows semantic versioning, but if it does, this should only be done in version 3.x
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts merged pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts merged pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-687820219
@NicholasNoise btw, I'd just like to update the build system in master to produce the sha512 sigs at build time, sign artifacts from appveyer, update the site and then I'll propose a vote to release 2.0.10 since it improves netstandard2.0 support for all users.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] cremor commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
cremor commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-692483473
> btw, this release is a bit messed up ([1](https://github.com/apache/logging-log4net/blob/master/src/log4net/AssemblyInfo.cs#L57), [2](https://github.com/apache/logging-log4net/blob/master/package.json#L3), #66), but no harm done.
About the wrong versions: I would suggest to set `GenerateAssemblyInfo` to true in the csproj to let the build create all those assembly attributes. Right now the version numbers are scattered over 3 files (6 if you count that weird cpp/js/vb files) which always has the risk of forgetting one.
In my opinion, the different informational versions and titles for the differnet target frameworks can be removed. You see the target framework in the `TargetFrameworkAttribute` anyway. But if you want to keep them I'm sure there is a way to do it in a single csproj tag using csproj variables.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691710323
>
>
> the more we do this, the better we get, right? At least, that's the plan (:
This is the way...
If it turns sideways we change plan (:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691513310
@NicholasNoise the vote passed, package 2.0.10 is up at nuget.org; the site should hopefully be updated when someone with the required permissions has some time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685695290
@fluffynuts Thank you for quick response!
I made this PR from non-personal fork, but already requested branch setting change to an admin of organization (should be granted by now)
1. I am not familiar with definition of log4net's build system and hardly understand how it really works. So better you than me :)
By default VS2019 doesnt support netcoreapp1.0, but sdk can be downloaded and installed manually ([link](https://dotnet.microsoft.com/download/dotnet-core/1.0))
Also I've dropped `net35-client` and `net40-client` as target frameworks, because unable to compile against. Should we bring them back and test somehow or deprecate and drop support (someone uses client profile this year?)?
2. `nupgk` output directory maybe customized by `PackageOutputPath` specified in csproj:
```
<PackageOutputPath>..\..\build\artifacts</PackageOutputPath>
```
I should mention that csproj has no support of some nuspec properties: `summary` was deprecated, `owners` was also deprecated and should be specified on nuget.org package page.
And generates additional framework-specific references:
```
<frameworkAssemblies>
<frameworkAssembly assemblyName="System.Configuration" targetFramework=".NETFramework2.0, .NETFramework3.5, .NETFramework4.0, .NETFramework4.5" />
<frameworkAssembly assemblyName="System.Web" targetFramework=".NETFramework2.0, .NETFramework3.5, .NETFramework4.0, .NETFramework4.5" />
</frameworkAssemblies>
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-687048905
>
>
> @NicholasNoise please have a look over the changes I've made to see that you're happy with them.
>
> Also, since I know building for the older targets can be a mission, I've added a helper script to get dotnet core 1.1 & updated the docs to draw attention to them. Please try the `install-net-framework-sdk-3.5.ps1` helper to see if that resolves your issue with not being able to build against Client Profile (net35 and net40). I last got my machine building against these targets some time ago, so it would help to have verification that the script I wrote then still works (:
Script works perfectly!
First time build had failed, I made so magic:
- remove `net35-client`
- build
- add `net35-client` back
- build
- profit!
Other changes are fine with me. Thank you!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685327198
@rgoers @fluffynuts
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691705746
btw, turned out _I_ have the required permissions... still new here :flushed: anyhoo, the more we do this, the better we get, right? At least, that's the plan (:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685935299
> Reflecting my previous experience adding or dropping targets are rarely considered as breaking\major change, minor at best.
it's breaking for people who depend on it; I'd (personally) like to drop client-profile targets, but I'm not ready to yet and I think that doing so would justify a major version increment
> As a reminder to @fluffynuts: better check content of nupgk before publishing it.
I did, but I forgot to check on client-profile outputs because I'm a derp. Now that I'm reminded of it, I remember _not_ seeing those outputs and I'll add them back in again.
Yes, it's a PITA to get the environment required to build all targets (I've not only done it more than once, but struggled to find off-premise CI to do so!) -- but AppVeyor can do it, so I guess perhaps the real solution might be to provide a non-CP .csproj to encourage contributions, at least for now, or to accept that (again, at least for now), this is a sticky situation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685695290
@fluffynuts Thank you for quick response!
I made this PR from non-personal fork, but already requested branch setting change to an admin of organization (should be granted by now)
1. I am not familiar with definition of log4net's build system and hardly understand how it really works. So better you than me :)
By default VS2019 doesnt support netcoreapp1.0, but sdk can be downloaded and installed manually ([link](https://dotnet.microsoft.com/download/dotnet-core/1.0))
Also I've dropped `net35-client` and `net40-client` as target frameworks, because unable to compile against. Should we bring them back and test somehow or deprecate and drop support (someone uses client profile this year?)?
2. `nupgk` output directory maybe customized by `PackageOutputPath` specified in csproj:
```
<PackageOutputPath>..\..\build\artifacts</PackageOutputPath>
```
I should mention that csproj has no support of some nuspec properties: `summary` was deprecated, `owners` was also deprecated and should be specified on nuget.org package page.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685726181
>
>
> I'd be very careful about dropping target frameworks. This is a breaking change. All projects using that framework will no longer be able to use the newer versions of log4net. I don't know if log4net follows semantic versioning, but if it does, this should only be done in version 3.x
Reflecting my previous experience adding or dropping targets are rarely considered as breaking\major change, minor at best.
But anyway I strongly agree that drop is not necessary, this change has been made out of my local environment.
As a reminder to @fluffynuts: better check content of nupgk before publishing it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] rgoers commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
rgoers commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-687820716
I would suggest the next release include the fix for the open CVE as it is confusing users as they are expecting it. My understanding is the the fix is on the develop branch but was never merged to master.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691525273
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-687850791
@rgoers good idea. It's in a commit on develop and I'm not sure what else is in there, so I'd like to simply cherry-pick that commit. I'll do so now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-686364191
>
> Yes, it's a PITA to get the environment required to build all targets (I've not only done it more than once, but struggled to find off-premise CI to do so!) -- but AppVeyor can do it, so I guess perhaps the real solution might be to provide a non-CP .csproj to encourage contributions, at least for now, or to accept that (again, at least for now), this is a sticky situation.
I'm thinking to add new step of PR checklist: is every target framework in place?
>
> It doesn't help that getting the SDK for these environments is such a mission because they are well beyond their support lifetime; again, I'd _like_ to drop support for unsupported frameworks (and that includes netapp1.(0|1)!), but that should come in a slightly less shocking format, I think.
Yep, I think that keeping project on LTS framework is the only way to keep it alive.
Breaking changes such as drop of previously supported frameworks should be planned, documented and consolidated: for example to drop support of netcoreapp1.0 as runtime we should drop netstandard1.*,
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691513310
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685442735
@NicholasNoise I really appreciate the PR & the time taken to produce multi-target tests! Good job.
There are two changes I'd like to make -- either we can collaborate (if you allow me to push to your PR branch -- https://stackoverflow.com/questions/56350103/how-do-i-allow-just-1-user-to-push-to-a-branch-in-github) or if @rgoers is happy, we can merge in and I'll sort them out later:
1. now that there are multi-target tests, `npm test` should invoke all of them. It's not that difficult to do -- or perhaps it just seems that way because I authored `zarro` too, so I know how it works. I'd like to check that these tests still run through AppVeyer (reminds me: I have to double-check that there's automatic build & test for this repo: I set up AppVeyer against my own fork originally, before I had write access here), for people who perhaps can't run all platform tests locally (I'm not sure if the vs2019 image there supports netcoreapp 1.0 -- I had to install the old sdk to run tests on my machine)
2. switching from the .nuspec to using inbuild nupkg building is the right thing to do -- but it breaks the `release` npm script which builds, tests, packs & zips to prepare all artifacts for release. I can see how to fix that by changing the pipeline a little as a release build produces the .nupkg just fine, but it does need to be sorted out.
all-in-all, I'm happy with the changes -- would just like to address the above. @rgoers since I'm still Quite New here, I'd appreciate your input on any preference as to how things get done?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-691525273
@fluffynuts thanks!
I'll test it in live environment and come back with a feedback.
btw, this release is a bit messed up ([1](https://github.com/apache/logging-log4net/blob/master/src/log4net/AssemblyInfo.cs#L57), [2](https://github.com/apache/logging-log4net/blob/master/package.json#L3), #66), but harm done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise commented on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-687859915
>
>
> @NicholasNoise btw, I'd just like to update the build system in master to produce the sha512 sigs at build time, sign artifacts from appveyer, update the site and then I'll propose a vote to release 2.0.10 since it improves netstandard2.0 support for all users.
Thanks!
Patiently (or maybe just a little impatiently) waiting the release. ETA?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685442735
@NicholasNoise I really appreciate the PR & the time taken to produce multi-target tests! Good job.
There are two changes I'd like to make -- either we can collaborate (if you allow me to push to your PR branch -- https://stackoverflow.com/questions/56350103/how-do-i-allow-just-1-user-to-push-to-a-branch-in-github) or if @rgoers is happy, we can merge in and I'll sort them out later:
1. now that there are multi-target tests, `npm test` should invoke all of them. It's not that difficult to do -- or perhaps it just seems that way because I authored `zarro` too, so I know how it works. I'd like to check that these tests still run through AppVeyer (reminds me: I have to double-check that there's automatic build & test for this repo: I set up AppVeyer against my own fork originally, before I had write access here), for people who perhaps can't run all platform tests locally (I'm not sure if the vs2019 image there supports netcoreapp 1.0 -- I had to install the old sdk to run tests on my machine)
2. switching from the .nuspec to using inbuild nupkg building is the right thing to do -- but it breaks the `release` npm script which builds, tests, packs & zips to prepare all artifacts for release. I can see how to fix that by changing the pipeline a little as a release build produces the .nupkg just fine, but it does need to be sorted out. Basically the `pack` task can be dropped and the `zip` task should be updated to move the .nupkg to the build/artifacts location before attempting to zip up binary & source artifacts.
all-in-all, I'm happy with the changes -- would just like to address the above. @rgoers since I'm still Quite New here, I'd appreciate your input on any preference as to how things get done?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] fluffynuts edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-685935299
> Reflecting my previous experience adding or dropping targets are rarely considered as breaking\major change, minor at best.
it's breaking for people who depend on it; I'd (personally) like to drop client-profile targets, but I'm not ready to yet and I think that doing so would justify a major version increment
> As a reminder to @fluffynuts: better check content of nupgk before publishing it.
I did, but I forgot to check on client-profile outputs because I'm a derp. Now that I'm reminded of it, I remember _not_ seeing those outputs and I'll add them back in again.
Yes, it's a PITA to get the environment required to build all targets (I've not only done it more than once, but struggled to find off-premise CI to do so!) -- but AppVeyor can do it, so I guess perhaps the real solution might be to provide a non-CP .csproj to encourage contributions, at least for now, or to accept that (again, at least for now), this is a sticky situation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4net] NicholasNoise edited a comment on pull request #63: Implemented proper support of netstandard2.0
Posted by GitBox <gi...@apache.org>.
NicholasNoise edited a comment on pull request #63:
URL: https://github.com/apache/logging-log4net/pull/63#issuecomment-686364191
>
> Yes, it's a PITA to get the environment required to build all targets (I've not only done it more than once, but struggled to find off-premise CI to do so!) -- but AppVeyor can do it, so I guess perhaps the real solution might be to provide a non-CP .csproj to encourage contributions, at least for now, or to accept that (again, at least for now), this is a sticky situation.
I'm thinking to add new step of PR checklist: is every target framework in place?
>
> It doesn't help that getting the SDK for these environments is such a mission because they are well beyond their support lifetime; again, I'd _like_ to drop support for unsupported frameworks (and that includes netapp1.(0|1)!), but that should come in a slightly less shocking format, I think.
Yep, I think that keeping project on LTS framework is the only way to keep it alive.
Breaking changes such as drop of previously supported frameworks should be planned, documented and consolidated: for example to drop support of netcoreapp1.0 as runtime we should drop netstandard1.*,
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org