Skip to content

Conversation

kapilmb
Copy link

@kapilmb kapilmb commented Sep 7, 2016

Fixes #552


This change is Reviewable

Kapil Borle added 2 commits September 6, 2016 20:30
The PSUseDeclaredVarsMoreThanAssignment rule did not check if a variable is being assigned and used in the same or nested scopes. This commit makes sure that a variable assigned in a scope is checked for its usage only in that scope or its children scopes.
@raghushantha
Copy link
Member

Rules/Strings.resx, line 163 [r1] (raw file):

  </data>
  <data name="UseDeclaredVarsMoreThanAssignmentsDescription" xml:space="preserve">
    <value>Checks that variables are used in more than just their assignment. Generally this is a red flag that a variable is not needed.</value>

Need to rephrase this:

"Ensure declared variables are used elsewhere in the script and not just during assignment"

How does this sound?


Comments from Reviewable

@raghushantha
Copy link
Member

Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@kapilmb
Copy link
Author

kapilmb commented Sep 7, 2016

Rules/Strings.resx, line 163 [r1] (raw file):

Previously, raghushantha (Raghu Shantha [MSFT]) wrote…

Need to rephrase this:

"Ensure declared variables are used elsewhere in the script and not just during assignment"

How does this sound?

I think your suggestions sound better. Will make the change.

Comments from Reviewable

@raghushantha
Copy link
Member

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@raghushantha
Copy link
Member

:lgtm:


Comments from Reviewable

@kapilmb
Copy link
Author

kapilmb commented Sep 7, 2016

Thanks @raghushantha !

@kapilmb kapilmb merged commit 1d6718c into development Sep 7, 2016
@kapilmb kapilmb deleted the FixUseDeclaredVarsMoreThanAssgn branch September 8, 2016 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants