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

remove some redundant variable assignments #6184

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

SimonCropp
Copy link
Contributor

Bug

Fixes:

Description

remove some redundant variable assignments

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@SimonCropp SimonCropp requested a review from a team as a code owner December 4, 2024 23:26
@microsoft-github-policy-service microsoft-github-policy-service bot added the Community PRs created by someone not in the NuGet team label Dec 4, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Dec 12, 2024
Copy link
Contributor

@Nigusu-Allehu Nigusu-Allehu left a comment

Choose a reason for hiding this comment

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

Thank you for your contributions! Out of curiosity and for learning purposes, did you identify these redundant assignments manually by reviewing all our code, or did you use a script to help with that? It seems like it would be a lot of effort to do manually, and I'm impressed either way!

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Dec 13, 2024
@SimonCropp
Copy link
Contributor Author

@Nigusu-Allehu i use a variety of techniques and tools. usually i just open random file and review them. often starting with the larger files. when i find a problem/improvement i work out some approach to finding any other instances of that problem/improvement. this approach can be varied: a test that uses roslyn, a regex search, features provided by an ide, etc

in this case it was relatively easy given rider has a feature that will find redundant variable assignments

Copy link
Member

@nkolev92 nkolev92 left a comment

Choose a reason for hiding this comment

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

This is one of those stylistic changes that while it's good, I think it shines best if there's a rule change.

@@ -70,7 +70,7 @@ public NuGetProject TryCreateNuGetProject(
// Treat projects with project.json as build integrated projects
// Search for projectName.project.json first, then project.json
// If the name cannot be determined, search only for project.json
string projectJsonPath = null;
string projectJsonPath;
Copy link
Member

Choose a reason for hiding this comment

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

I'd feel more strongly about accepting this PR if it was coupled with a rule change as well: https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0059.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community PRs created by someone not in the NuGet team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants