-
Notifications
You must be signed in to change notification settings - Fork 232
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
Update RSPEC before 9.23 release #8976
Conversation
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
All changes have a clear origin, and are correct.
@@ -9,8 +9,8 @@ <h3>Exceptions</h3> | |||
<li> literals used in attributes </li> | |||
</ul> | |||
<h2>How to fix it</h2> | |||
<p>Instead, use constants to replace the duplicated string literals. Constants can be referenced from many places, but only need to be updated in a | |||
single place.</p> | |||
<p>Use constants to replace the duplicated string literals. Constants can be referenced from many places, but only need to be updated in a single |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is related to this PR: SonarSource/rspec#3789
While the change was made in the context of IaC, it is acceptable also in the .NET context too.
@@ -9,8 +9,8 @@ <h3>Exceptions</h3> | |||
<li> literals used in attributes </li> | |||
</ul> | |||
<h2>How to fix it</h2> | |||
<p>Instead, use constants to replace the duplicated string literals. Constants can be referenced from many places, but only need to be updated in a | |||
single place.</p> | |||
<p>Use constants to replace the duplicated string literals. Constants can be referenced from many places, but only need to be updated in a single |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
a mistake and try to 'fix' the code, potentially introducing new bugs. </li> | ||
<li> Memory Usage: Although modern compilers are smart enough to ignore unused variables, not all compilers do this. In such cases, unused variables | ||
take up memory space, leading to inefficient use of resources. </li> | ||
<li> <strong>Decreased Readability</strong>: Unused variables can make your code more difficult to read. They add extra lines and complexity, which |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change was done by @jonas-wielage-sonarsource for IaC in the context of SonarSource/rspec#3790.
The change is only about formatting, and it's OK for both C# and VB.NET.
@@ -27,7 +27,7 @@ <h4>Compliant solution</h4> | |||
<pre data-diff-id="1" data-diff-type="compliant"> | |||
bool IsDefault<T>(T value) | |||
{ | |||
if(object.Equals(value, default(T))) | |||
if (EqualityComparer<T>.Default.Equals(value, default(T))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change has been done by @gregory-paidis-sonarsource in the context of SonarSource/rspec#3794.
I have added this rule to the list of rules changed without RSPEC update, in the issue.
Fixes #8913