- Notifications
You must be signed in to change notification settings - Fork407
Fix logic errors in AvoidAlias rule#1251
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fix logic errors in AvoidAlias rule#1251
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The first fix is that after the yield return, we should move on to the next commandThe second fix is that if we find _any_ command, we should not move on to the 'Get-<command>' variantThese changes mean that we do not run a pipeline (twice!) to find something that was a cache miss. In the pathological case of a script mmade up of only unique aliases, we would run 2 pipelines for each found alias.
Uh oh!
There was an error while loading.Please reload this page.
bergmeister left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
One minor suggestion but looks great otherwise. I will modify PR#1178 to not parallelize the AvoidAlias rule any more since with this improvement the rule is no longer the weakest link in the chain
Co-Authored-By: Christoph Bergmeister [MVP] <c.bergmeister@gmail.com>
PR Summary
The first fix is that after the yield return, we should move on to the next command
The second fix is that if we findany command, we should not move on to the 'Get-' variant.
These changes mean that we do not run a pipeline (twice!) to find something that was a cache miss. Previously in the pathological case of a script made up of only unique aliases, we would run 2 pipelines for each found alias. This fix avoids both of those pipeline invocations.
PR Checklist
.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.