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 IDE0059: Value assigned to variable is never used #3155

Merged

Conversation

vgromfeld
Copy link
Contributor

PR Type

What kind of change does this PR introduce?
Code style update (formatting)

What is the current behavior?

The current code base contains a lot of IDE0059: Value assigned to variable is never used error messages.
Some are styling issues like

int i = 0;
i = 2;

Others are related to unused objects being allocated like:

var keyTimeFromAnimationDuration = KeyTime.FromTimeSpan(ts);
[...]
private JsonSerializer serializer = new JsonSerializer();

Some others are related to unused out parameters/returned variables which are not explicitly discarded. See Discards.

All those unused variables are not needed and will consumed memory and/or perform useless computation. They can be removed.

What is the new behavior?

After this change, the code behavior is exactly the same. I've removed the unused variables and use the discard operator where needed.
I've also remove two unused methods.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • [ ] Header has been added to all new source files (run build/UpdateHeaders.bat)
  • [ ✔ ] Contains NO breaking changes

Other information

@ghost
Copy link

ghost commented Feb 28, 2020

Thanks vgromfeld for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost assigned michael-hawker Feb 28, 2020
@dnfclas
Copy link

dnfclas commented Feb 28, 2020

CLA assistant check
All CLA requirements met.

@dnfclas
Copy link

dnfclas commented Feb 28, 2020

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ vgromfeld sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

@Kyaa-dost
Copy link
Contributor

@vgromfeld Here is what we see. The build is failing.

DataGrid\DataGridDataConnection.cs(546,26): error SA1513: Closing brace must be followed by blank line [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.csproj]
DataGrid\DataGridDataConnection.cs(547,25): error SA1515: Single-line comment must be preceded by blank line [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid\Microsoft.Toolkit.Uwp.UI.Controls.DataGrid.csproj]

Rss\RssHelper.cs(377,62): error SA1311: Static readonly fields must begin with upper-case letter [D:\a\1\s\Microsoft.Toolkit.Parsers\Microsoft.Toolkit.Parsers.csproj]

Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

Thanks @vgromfeld for this clean-up! Couple of minor questions, and also @Kyaa-dost pointed out some other style validations which prevent the build CI from completing. Otherwise, looking great!

Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

Found more about the handled question, think that's my only item left. :)

@ghost
Copy link

ghost commented Mar 13, 2020

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

@michael-hawker
Copy link
Member

@azchohfi think we're all good here, eh?

@vgromfeld vgromfeld requested a review from azchohfi March 18, 2020 08:28
@ghost
Copy link

ghost commented Mar 19, 2020

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

@vgromfeld vgromfeld merged commit 5e030db into CommunityToolkit:master Mar 23, 2020
@michael-hawker
Copy link
Member

🎉 Thanks @vgromfeld for this great clean-up work!

@michael-hawker michael-hawker added this to the 6.1 milestone Mar 23, 2020
@michael-hawker michael-hawker mentioned this pull request Mar 27, 2020
36 tasks
@Kyaa-dost Kyaa-dost added bug 🐛 An unexpected issue that highlights incorrect behavior and removed needs attention 👋 labels Jun 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants