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/08 08:59:24 UTC
[GitHub] [logging-log4net] NicholasNoise opened a new pull request #67: Add FxCop.
NicholasNoise opened a new pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67
Fix CA2000, CA2237, CA3075
----------------------------------------------------------------
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 #67: Add FxCop.
Posted by GitBox <gi...@apache.org>.
NicholasNoise edited a comment on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694069775
@fluffynuts Every warning you see may be (or should be) turned off by ruleset: some warning are essential and should be attended to, others are not and just ignore them with no regret.
FxCop grants build validation, so no one will be able to push "bad" code. Standalone tools like SonarQube are good, but runs manually, periodically or by event-based triggers and generated results should be analyzed by someone. Once I saw generated report - like looking into spam or notification category in my mail box.
Anyway you are right that pushing a warning-free project into the dirt is no good.
I think to split this PR into 2 parts:
- Fix CA2000, CA2237, CA3075 - this pr
- Add FxCop - new pull request with ruleset and csproj configuration changes.
Are you ok with 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] fluffynuts commented on pull request #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-697159824
@NicholasNoise 2.0.11 is out with these changes. Things take a little while because:
1. I hadn't had my gpg key accepted into the apache KEYS file yet
2. The process involves a vote, all from people who are doing this in their own time
3. I'm basically the only committer to this project, for now. Of course, I'm interested in helping other people to join me if they want to (:
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-695350068
> > StyleCop isn't enough
>
> StyleCop and FxCop grant a build validation with right ruleset (SA1027 set to Error), so it is IDE independent.
What I mean is that StyleCop is a "you broke it, so you need to fix it" solution -- a gate, if you will, which is good, but better is something which informs the editor of the correct choice to make so that when the coder presses <tab>, _the right thing is done_ and the coder doesn't have to retroactively fix things or update their IDE prefs to fit in with StyleCop. EditorConfig provides this.
IOW, StyleCop is retroactive where editorconfig is proactive. Keep the retroactive to enforce style, bit provide the proactive to make it not the problem of the new contributor to get it right (:
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694165275
>
>
> That sounds like a good plan 👍
Done!
> I don't know enough about FxCop to be sure, but I'd expect that we could perhaps enable rule processing for a specific build configuration only -- and have that build configuration targeted by `npm test`, which can be run by CI (I have it running against my own appveyer, but haven't yet set it up for this official repository -- yet another Thing To Get Done).
Two ruleset is the way to get things done (:
> Don't be afraid of an external tool though -- it can always be made part of the build pipeline. That's not a bump for or against any particular tool -- just a note.
Build-in tools are preferable, an external tool makes it harder for new contributor to join. But in the same time used tools should be effective by all means.
I'm not familiar with SonarQube to make any conclusion so it is not up to me :)
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-697160161
@NicholasNoise if you're interested in (3), let me know how to contact you. Your GitHub profile has no contact info ):
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-695350068
> > StyleCop isn't enough
>
> StyleCop and FxCop grant a build validation with right ruleset (SA1027 set to Error), so it is IDE independent.
What I mean is that StyleCop is a "you broke it, so you need to fix it" solution -- a gate, if you will, which is good, but better is something which informs the editor of the correct choice to make so that when the coder presses <tab>, _the right thing is done_ and the coder doesn't have to retroactively fix things or update their IDE prefs to fit in with StyleCop. EditorConfig provides this.
IOW, StyleCop is retroactive where editorconfig is proactive. Keep the retroactive to enforce style, bit provide the proactive to make it not the problem of the new contributor to get it right (:
Editorconfig was enough to get my team all on the same page across win/osx for another project - no errors or warnings, just editors fixing styles as people worked.
Anyway, it's all academic until I get some time to add a .editorconfig. I would welcome a StyleCop PR though - I don't use it, so wouldn't know offhand how to set it up. My opinion is that we should be helping contributors to make meaningful contributions with as little resistance as possible.
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-695349836
I've started a vote, have one +1, so waiting on 2 more.
----------------------------------------------------------------
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 a change in pull request #67: Add FxCop.
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on a change in pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#discussion_r484872716
##########
File path: src/log4net/Appender/FileAppender.cs
##########
@@ -1409,7 +1429,10 @@ virtual protected void OpenFile(string fileName, bool append)
/// </remarks>
virtual protected void SetQWForFiles(Stream fileStream)
{
- SetQWForFiles(new StreamWriter(fileStream, m_encoding));
+#pragma warning disable CA2000 // Dispose objects before losing scope
Review comment:
I think we should refactor this somehow.
----------------------------------------------------------------
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 #67: Add FxCop.
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-688829338
@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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694207456
@NicholasNoise I need to ask you for one more change before merging: the style of the project has been to use spaces for indentation. I noticed some whitespace changes on the prior PRs and just left it, but I'd really appreciate it if you could use spaces instead of tabs in this project. Perhaps I should set up an .editorconfig to assist with this...
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694750050
I prefer spaces too by its _looks-the-same-everywhere_ feature.
> StyleCop isn't enough
StyleCop and FxCop grant a build validation with right ruleset (SA1027 set to Error), so it is IDE independent.
IDE provides valid underlining for an incorrect tabulation. It is my favorite way to force things right.
> I think I'll just accept what you have here
Thanks!
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-697184642
> 2.0.11 is out with these changes.
Thanks!
> Things take a little while
There is no problem at all, I've saw insides of how-to-release-project - is not that simple as it looks.
@fluffynuts
I contribute open source projects on a non-regular basic, but for now.
Anyway you can contact me by mailing me at gmail.com (:
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694206130
> Build-in tools are preferable, an external tool makes it harder for new contributor to join. But in the same time used tools should be effective by all means.
> I'm not familiar with SonarQube to make any conclusion so it is not up to me :)
I'm not talking about expecting people to install stuff -- external tools which can be bootstrapped by the build pipeline are ok -- for instance, running tests: for the primary run, we use the nunit runner, but how that happens is all tucked away in the bowels of `zarro test-dotnet`, which will download the nunit console runner nuget package & use it to run tests. Same goes for coverage via dotCover or OpenCover (and subsequent report generation with ReportGenerator, if using OpenCover). Where I can't just automate it, I try to provide helpers like the .ps1 scripts in the root of the repo to get more esoteric build targets installed. But for a new dev, if one has node & the relevant .net tools installed (eg vs build tools install), then
```
npm ci
npm release
```
will
- bootstrap all required tooling
- restore packages
- build
- test
- generate build artifacts
At some point, I want to automate the site into that (I have a `build-site` npm script which I use, just not 100% convinced it should be part of the build pipeline yet because `release` can be run at a CI server which wouldn't (shouldn't) be able to push back the generated site). Also need to fix up the sdk docs which went missing, and then that should be a part of the `build-site` pipeline.
The point is: I don't mind external stuff which can be automated. I try to reduce non-automatable dependencies, which should be documented (yes, I have work to do there too, in cleaning up the docs).
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts merged pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694206130
> Build-in tools are preferable, an external tool makes it harder for new contributor to join. But in the same time used tools should be effective by all means.
> I'm not familiar with SonarQube to make any conclusion so it is not up to me :)
I'm not talking about expecting people to install stuff -- external tools which can be bootstrapped by the build pipeline are ok -- for instance, running tests: for the primary run, we use the nunit runner, but how that happens is all tucked away in the bowels of `zarro test-dotnet`, which will download the nunit console runner nuget package & use it to run tests. Same goes for coverage via dotCover or OpenCover (and subsequent report generation with ReportGenerator, if using OpenCover). Where I can't just automate it, I try to provide helpers like the .ps1 scripts in the root of the repo to get more esoteric build targets installed. But for a new dev, if one has node & the relevant .net tools installed (eg vs build tools install), then
```
npm ci
npm release
```
will
- bootstrap all required tooling
- restore packages
- build
- test
- generate build artifacts
At some point, I want to automate the site into that (I have a `build-site` npm script which I use, just not 100% convinced it should be part of the build pipeline yet because `release` can be run at a CI server which wouldn't (shouldn't) be able to push back the generated site). Also need to fix up the sdk docs which went missing, and then that should be a part of the `build-site` pipeline.
The point is: I don't mind external stuff which can be automated. I try to reduce non-automatable dependencies, which should be documented (yes, I have work to do there too, in cleaning up the docs).
----------------------------------------------------------------
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 #67: Add FxCop.
Posted by GitBox <gi...@apache.org>.
NicholasNoise edited a comment on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694069775
@fluffynuts Every warning you see may be (or should be) turned off by ruleset: some warning are essential and should be attended to, others are not and just ignore them with no regret.
FxCop grants build validation, so no one will be able to push "bad" code. (FYI: FxCop is build-in tool in dotnet 5) Standalone tools like SonarQube are good, but runs manually, periodically or by event-based triggers and generated results should be analyzed by someone. Once I saw generated report - like looking into spam or notification category in my mail box.
Anyway you are right that pushing a warning-free project into the dirt is no good.
I think to split this PR into 2 parts:
- Fix CA2000, CA2237, CA3075 - this pr
- Add FxCop - new pull request with ruleset and csproj configuration changes.
Are you ok with 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] NicholasNoise edited a comment on pull request #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
NicholasNoise edited a comment on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694248929
> the style of the project has been to use spaces for indentation.
One more reason to use something like StyleCop to prevent from happening.
Actually the style of log4net is tab-based indent for 90% of files.
But if you say so I can convert changed files.
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-699035185
> There is no problem at all, I've saw insides of how-to-release-project - is not that simple as it looks.
yeah, I need to update that too, because quite a bit of helpful contributor documentation is out of date ):
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694248929
> the style of the project has been to use spaces for indentation.
One more reason to use something like StyleCop to prevent from happening.
Actually the style of log4net is tab-based indent for 90% of files.
----------------------------------------------------------------
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 #67: Add FxCop.
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694097316
btw, I don't have anything against FxCop, and I expect rules would have to be established, just like for SonarQube; I guess I'm just remembering how successful SonarQube was for me on one of my projects because I could chip away at warnings one by one until I got a clean bill of health without interfering with build elsewhere. I don't know enough about FxCop to be sure, but I'd expect that we could perhaps enable rule processing for a specific build configuration only -- and have that build configuration targeted by `npm test`, which can be run by CI (I have it running against my own appveyer, but haven't yet set it up for this official repository -- yet another Thing To Get Done).
Don't be afraid of an external tool though -- it can always be made part of the build pipeline. That's not a bump for or against any particular tool -- just a note.
----------------------------------------------------------------
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 #67: Add FxCop.
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694069775
@fluffynuts Every warning you see may be (or should be) turned off by ruleset: some warning are essential and should be attended to, others are not and just ignore them with no regret.
FxCop grants build validation, so no one will be able to push "bad" code. Standalone tools like SonarQube are good, but runs manually, periodically or by event-based triggers and generated results should be analyzed by someone. Once I saw generated report - like looking into spam or notification category in my mail box.
Anyway you are right that pushing a warning-free project into the dirt is no good.
I think to split this PR into 2 parts:
- Fix CA2000, CA2237, CA3075 -
- Add FxCop - new pull request with ruleset and csproj configuration changes.
Are you ok with 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] fluffynuts commented on pull request #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-695350068
> > StyleCop isn't enough
>
> StyleCop and FxCop grant a build validation with right ruleset (SA1027 set to Error), so it is IDE independent.
What I mean is that StyleCop is a "you broke it, so you need to fix it" solution -- a gate, if you will, which is good, but better is something which informs the editor of the correct choice to make so that when the coder presses <tab>, _the right thing is done_ and the coder doesn't have to retroactively fix things or update their IDE prefs to fit in with StyleCop. EditorConfig provides this.
----------------------------------------------------------------
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 #67: Add FxCop.
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-693981771
@NicholasNoise I'm all for trying to up code quality; this adds a _ton_ of noise though. For me, warnings are a thing that should not be seen -- all warnings should be attended to, either by solving the problem the warning is about or by intentionally suppressing the warning. Perhaps FxCop isn't the way to go? Perhaps we should look at something like SonarQube? It's equally noisy, but not in the build.
The problem with a sea of warnings is that no-one will deal with them, and they drown out potentially problematic stuff. For instance, you've fixed up a place where a StreamWriter wasn't disposed (good!), but how would I spot that in all of this if you hadn't? I _really want_ all the fixes for stuff which hasn't been disposed, but I'm a little overwhelmed by the new warning noise.
Also, FYI, `npm run build` fails:
```
Util\PropertiesDictionary.cs(39,22): error CA2237: Add [Serializable] to PropertiesDictionary as this type implements ISerializable [C:\code\opensource\log4net\src\log4net\log4net.csproj]
```
Took me a while to find it in the swathes of warnings.
----------------------------------------------------------------
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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694708069
hm, I don't mind, really, what style is adopted -- in the file where I saw whitespace changes, it looked like the original code was spaces and your changes were tabs. Now I'd like to get an overview: whilst my preference is for spaces in C# (it's the default style, and I'm about adopting default styles instead of bucking them), I understand how tabs can be beneficial to the few people who care to set up configuration for them (:
StyleCop isn't enough -- a contributor may use something other than Visual Studio or a ReSharper-aware IDE (personally, I use Rider, but there's a case for VSCode, or whatever someone wants to use) to edit code. Most decent editors should understand .editorconfig and assist the user to maintain correct style without them having to respond to warnings. I'm sure StyleCop can do more too, so perhaps it could be used in conjunction with an .editorconfig. There are a lot of conventions which can be configured via .editorconfig, in addition to simpler things like indentation: https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-code-style-settings-reference?view=vs-2019
I think I'll just accept what you have here (because the fixes are good) and mull over (a) what the style should be and (b) how to help to enforce 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] NicholasNoise commented on pull request #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
NicholasNoise commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694766793
@fluffynuts
Is 2.0.11 planned to release any time soon?
----------------------------------------------------------------
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 #67: Add FxCop.
Posted by GitBox <gi...@apache.org>.
fluffynuts commented on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-694094932
That sounds like a good 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 #67: Fix CA2000, CA2237, CA3075
Posted by GitBox <gi...@apache.org>.
fluffynuts edited a comment on pull request #67:
URL: https://github.com/apache/logging-log4net/pull/67#issuecomment-695350068
> > StyleCop isn't enough
>
> StyleCop and FxCop grant a build validation with right ruleset (SA1027 set to Error), so it is IDE independent.
What I mean is that StyleCop is a "you broke it, so you need to fix it" solution -- a gate, if you will, which is good, but better is something which informs the editor of the correct choice to make so that when the coder presses <tab>, _the right thing is done_ and the coder doesn't have to retroactively fix things or update their IDE prefs to fit in with StyleCop. EditorConfig provides this.
IOW, StyleCop is retroactive where editorconfig is proactive. Keep the retroactive to enforce style, bit provide the proactive to make it not the problem of the new contributor to get it right (:
Anyway, it's all academic until I get some time to add a .editorconfig. I would welcome a StyleCop PR though - I don't use it, so wouldn't know offhand how to set it up. My opinion is that we should be helping contributors to make meaningful contributions with as little resistance as possible.
----------------------------------------------------------------
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