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

Fix PSSA settings discovery + pick up changes to settings file #1206

Merged
merged 11 commits into from
Feb 26, 2020

Conversation

rjmholt
Copy link
Collaborator

@rjmholt rjmholt commented Feb 25, 2020

@bergmeister
Copy link
Collaborator

bergmeister commented Feb 25, 2020

Some code comments but I can already confirm that this fixes the problem of not seeing PSSA warnings when there is no settings . Thanks for taking the time :-)

@rjmholt rjmholt marked this pull request as ready for review February 25, 2020 20:31
Copy link
Collaborator

@bergmeister bergmeister left a comment

Choose a reason for hiding this comment

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

Code looks ok, I can re-test tomorrow again if the fix is still working after the last commits if you want to?

@rjmholt
Copy link
Collaborator Author

rjmholt commented Feb 25, 2020

I can re-test tomorrow again if the fix is still working after the last commits if you want to?

Yes please.

I've tested (and stepped through in the debugger) all the scenarios I could think of:

  • Default setting
  • Empty string
  • File not there
  • File deleted

But could always use more eyes

Co-Authored-By: Tyler James Leonhardt <tylerl0706@gmail.com>
@TylerLeonhardt
Copy link
Member

We don't need the FileSystemWatcher if we still check if the file exists on every invoke. Refactor to not have file watcher.

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@bergmeister bergmeister left a comment

Choose a reason for hiding this comment

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

Re-tested with latest changes ✅

@rjmholt rjmholt merged commit cb9898a into PowerShell:master Feb 26, 2020
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.

PSScriptAnalyzer not running when no settings file present
3 participants