Skip to content

Conversation

kapilmb
Copy link

@kapilmb kapilmb commented Aug 1, 2016

This change is Reviewable

charlieschmidt and others added 3 commits July 22, 2016 10:05
* External/Custom rule supression

* Revert to the previous GetExternalRecord() call on all external rules, rather than 1-by-1 to continue to take advantage of the runspacepool stuff.
New SupressRule() function that takes a single DiagnosticRecord and list of suppressions - bounces the RuleName of the record against that list.
@raghushantha
Copy link
Member

build.ps1, line 113 [r2] (raw file):

}

# Appyeyor errors out due to $profile being null. Hence...

Appveyor


Comments from Reviewable

@raghushantha
Copy link
Member

build.ps1, line 114 [r2] (raw file):

# Appyeyor errors out due to $profile being null. Hence...
$moduleRootPath = "$HOME/Documents/WindowsPowerShell/Modules"

some education for me: Why are we referring/hardcoding to the module path in user documents and not in programfiles or other module locations?


Comments from Reviewable

@raghushantha
Copy link
Member

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


Comments from Reviewable

@kapilmb
Copy link
Author

kapilmb commented Aug 2, 2016

build.ps1, line 114 [r2] (raw file):

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

some education for me: Why are we referring/hardcoding to the module path in user documents and not in programfiles or other module locations?

Because modifying user documents don't require admin privileges and I think we should make admin-required changes only when necessary. The purpose of installing PSSA to user documents modules is mostly for testing purposes, which doesn't need a system wide availability of PSSA, and hence we put it in user documents module.

Comments from Reviewable

@kapilmb
Copy link
Author

kapilmb commented Aug 2, 2016

build.ps1, line 114 [r2] (raw file):

Previously, kapilmb (Kapil Borle) wrote…

Because modifying user documents don't require admin privileges and I think we should make admin-required changes only when necessary. The purpose of installing PSSA to user documents modules is mostly for testing purposes, which doesn't need a system wide availability of PSSA, and hence we put it in user documents module.

Plus the hard-coding is actually a fallback if the profile variable is null

Comments from Reviewable

@raghushantha
Copy link
Member

Reviewed 1 of 1 files at r3.
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 Aug 2, 2016

@raghushantha Thanks!

@kapilmb kapilmb merged commit 83779a2 into development Aug 2, 2016
@kapilmb kapilmb deleted the ExternalRuleSuppression branch August 2, 2016 18:13
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.

4 participants