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

Fixed Sonar Lint code analysis warnings #2389

Merged
merged 1 commit into from
Nov 18, 2017

Conversation

snakefoot
Copy link
Contributor

And cleaned up PropertyHelper.TryImplicitConversion (Turned ugly with NetStandard1.5)

@snakefoot snakefoot force-pushed the SonarLintCodeAnalysis branch 2 times, most recently from 125b383 to bff9fbd Compare November 14, 2017 22:44
@snakefoot snakefoot changed the title Fixed Sonar Lint code analysis warnings, Fixed Sonar Lint code analysis warnings Nov 14, 2017
@codecov
Copy link

codecov bot commented Nov 14, 2017

Codecov Report

Merging #2389 into master will decrease coverage by <1%.
The diff coverage is 80%.

@@           Coverage Diff           @@
##           master   #2389    +/-   ##
=======================================
- Coverage      82%     82%   -<1%     
=======================================
  Files         318     318            
  Lines       22796   22805     +9     
  Branches     2789    2790     +1     
=======================================
- Hits        18671   18665     -6     
- Misses       3419    3433    +14     
- Partials      706     707     +1

Copy link
Member

@304NotModified 304NotModified left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this a pure refactoring, isn't? (so no functional changes and just moves etc?)

PS: AFAIK sonar isn't working correctly on AppVeyor

private Stack<object> _stack;
private int _previousCount;
private readonly Stack<object> _stack;
private readonly int _previousCount;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that readonly could influence runtime performance (badly)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only heard that it affect performance for fields in struct, where it will cause the entire struct to be copied on field access. Do you have some details about this other readonly pitfall?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@304NotModified 304NotModified added this to the 4.5 beta 8 milestone Nov 17, 2017
@304NotModified 304NotModified merged commit fa55b52 into NLog:master Nov 18, 2017
@snakefoot snakefoot modified the milestones: 4.5 beta 8, 4.5 Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants