You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Initially, I wanted to just log a warning but during the PR it was suggested/decided to rather return an object of type DiagnosticRecord to still alert the user about it in some way. It is therefore a bit unfortunate/misleading because it is not a rule per se and can therefore not be suppressed. However, because it is PowerShell you can filter out those DiagnosticRecords from the list of returned results. I myself am not 100% happy with this solution but this is how it works at the moment, sorry that it is a bit confusing.
Ok that's what I though but I wasn't completely sure.
I agree that ideally we should be able to treat it like any other rule violation.
But it's ok, I'll filter it afterwards like you mentioned.
It's hugely frustrating that it overrides whatever settings or suppression statements the user indicates. Even something such as this, would still see these "informational" items.
Obviously when you specify you only want warnings, and you see info records returned too, it is hugely detrimental to one's confidence in the product. Not everyone is going to come hunt through Github issues and learn that this is on purpose.
If I want to use PSSA in a CI/CD pipeline, and stop it on violations, am I just not supposed to use PSSA on files that have classes? Is the expectation that I'll filter these out myself? This is the wrong user experience.
Reacted by Matěj Pokorný, Jake Morrison, Jeremy, Johan Ljunggren, iRon7 and quasarea
Ok, gotcha, I guess it is better to convert them to PowerShell warnings instead of returning an object, I wasn't a fan of that approach either when we did the PR but the behaviour in 1.16 was worse because the cmdlet re-threw the parser exceptions.
That'd be way better, Christoph Bergmeister (@bergmeister) . The diagnostic record makes it look like PSSA is reporting the error and that leads a user to think they can use PSSA to address it. Instead they're left with this record they can't do anything about. If there's something I can upvote to change the behavior, please do let me know. :)
Thomas Rayner (@thomasrayner) Upvoting helps with prioritising a bit but in general, one just needs to open a PR if change is wanted. I opened PR 1126 for that since this is an easy fix that I had in the back of my mind anyway (but there's just so many other things)
Thomas Rayner (@thomasrayner) Don't worry, PR 1126 is ready now with a fix for it, it is just pending a CI problem on Linux due to an AppVeyor image update.
Just an update: PR #1126 had to be abandoned as the approach of just outputting a warning was not acceptable. New efforts using a different approach are happening in PR #1130 now
Activity
bergmeister commentedon Jul 19, 2018
Initially, I wanted to just log a warning but during the PR it was suggested/decided to rather return an object of type DiagnosticRecord to still alert the user about it in some way. It is therefore a bit unfortunate/misleading because it is not a rule per se and can therefore not be suppressed. However, because it is PowerShell you can filter out those DiagnosticRecords from the list of returned results. I myself am not 100% happy with this solution but this is how it works at the moment, sorry that it is a bit confusing.
avalery commentedon Jul 19, 2018
Ok that's what I though but I wasn't completely sure.
I agree that ideally we should be able to treat it like any other rule violation.
But it's ok, I'll filter it afterwards like you mentioned.
Thank you for your quick answer!
thomasrayner commentedon Jan 14, 2019
It's hugely frustrating that it overrides whatever settings or suppression statements the user indicates. Even something such as this, would still see these "informational" items.
Invoke-ScriptAnalyzer -Path $file -Severity WarningObviously when you specify you only want warnings, and you see info records returned too, it is hugely detrimental to one's confidence in the product. Not everyone is going to come hunt through Github issues and learn that this is on purpose.
If I want to use PSSA in a CI/CD pipeline, and stop it on violations, am I just not supposed to use PSSA on files that have classes? Is the expectation that I'll filter these out myself? This is the wrong user experience.
bergmeister commentedon Jan 14, 2019
Ok, gotcha, I guess it is better to convert them to PowerShell warnings instead of returning an object, I wasn't a fan of that approach either when we did the PR but the behaviour in 1.16 was worse because the cmdlet re-threw the parser exceptions.
thomasrayner commentedon Jan 14, 2019
That'd be way better, Christoph Bergmeister (@bergmeister) . The diagnostic record makes it look like PSSA is reporting the error and that leads a user to think they can use PSSA to address it. Instead they're left with this record they can't do anything about. If there's something I can upvote to change the behavior, please do let me know. :)
bergmeister commentedon Jan 15, 2019
Thomas Rayner (@thomasrayner) Upvoting helps with prioritising a bit but in general, one just needs to open a PR if change is wanted. I opened PR 1126 for that since this is an easy fix that I had in the back of my mind anyway (but there's just so many other things)
thomasrayner commentedon Jan 15, 2019
Thanks, Christoph Bergmeister (@bergmeister) ! I may take a look if I can make time for it, if that's not overstepping.
bergmeister commentedon Jan 15, 2019
Thomas Rayner (@thomasrayner) Don't worry, PR 1126 is ready now with a fix for it, it is just pending a CI problem on Linux due to an AppVeyor image update.
bergmeister commentedon Jan 29, 2019
Just an update: PR #1126 had to be abandoned as the approach of just outputting a warning was not acceptable. New efforts using a different approach are happening in PR #1130 now