[GitHub] [logging-log4net] fluffynuts edited a comment on pull request #63: Implemented proper support of netstandard2.0

[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