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

Fixed case on file names, and applied a new rules from script analyzer #590

Merged
merged 3 commits into from
May 1, 2019

Conversation

jhoneill
Copy link
Contributor

@jhoneill jhoneill commented May 1, 2019

Very little functional change - remove-worksheet supports -confirm, and I changed 3 or 4 instances of ::new() to new-object because that will work with PS 4 (we don't expressly want to support 4, but at the same time there's no sense in breaking 4 if we don't need to)
I've made some changes to either bring the code into line with PSScriptAnalyzer 1.18 guidlines or to tell it where it doesn't need to worry.
Also fixed a test which didn't work in the appveryor environment.

@dfinke
Copy link
Owner

dfinke commented May 1, 2019

Did the new PSSA catch the casing?

@dfinke dfinke merged commit e7d2b52 into dfinke:master May 1, 2019
@jhoneill
Copy link
Contributor Author

jhoneill commented May 1, 2019

No, it didn't. I installled the module in cloud-shell and 3 or 4 files were not found. All the changes outside the PSM1 came from PSSA. I've left a couple of the things it doesn't like for now. Your if ($x = do-something) trick gets queried, and I didn't want to disable checks for that, and there is at least one place where there is a plural name which could be re-thought but I'm leaving alone for now.

@dfinke
Copy link
Owner

dfinke commented May 2, 2019

Might be a useful report to run on commits

$xlfile = "$env:TEMP\ScriptAnalyzer.xlsx"
Remove-Item $xlfile -ErrorAction SilentlyContinue

$p = @{
    AutoSize          = $true
    AutoFilter        = $true
    Show              = $true
    IncludePivotTable = $true
    IncludePivotChart = $true
    Activate          = $true
    PivotRows         = 'Severity', 'RuleName'
    PivotData         = @{RuleName = 'Count' } 
    ChartType         = 'BarClustered'
}

Invoke-ScriptAnalyzer . | Export-Excel $xlfile @p

Before

image

After

image

@jhoneill
Copy link
Contributor Author

jhoneill commented May 2, 2019

@dfinke Genius ! The highest praise I can offer it is to put it straight into my presentation for later ... plagiarism is the highest form of flattery :-)

@dfinke
Copy link
Owner

dfinke commented May 2, 2019

Need to find a different word :-)

You don't do that! Please amplify. Probably will tweet this.

Plagiarism is the "wrongful appropriation" and "stealing and publication" of another author's "language, thoughts, ideas, or expressions" and the representation of them as one's own original work.

@jhoneill
Copy link
Contributor Author

jhoneill commented May 2, 2019

@dfinke

Research

(see Tom Lehrer)

@jhoneill
Copy link
Contributor Author

jhoneill commented May 3, 2019

@dfinke I've checked in an example named analyze-this.ps1
cd ..\GitHub\ImportExcel\

"." , (dir 'C:\Program Files\WindowsPowerShell\Modules\ImportExcel') | .\examples\ScriptAnalyzer\Analyze_this.ps1

produces this which compares the quality over versions
image

@dfinke
Copy link
Owner

dfinke commented May 4, 2019

Thanks! Was going to ask. I'll check in the the other too.

@dfinke
Copy link
Owner

dfinke commented May 5, 2019

Hmm. Don't see it.

@ili101 ili101 mentioned this pull request Oct 7, 2019
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.

2 participants