Skip to content
This repository has been archived by the owner on Aug 22, 2022. It is now read-only.

Code cleanup #172

Merged
merged 10 commits into from Jul 31, 2018
Merged

Code cleanup #172

merged 10 commits into from Jul 31, 2018

Conversation

it-praktyk
Copy link
Collaborator

No description provided.

@it-praktyk
Copy link
Collaborator Author

The style rules can be checked using tests like below - but it should be made before merging to the Release
Tests are from Pester.

Describe 'Style rules' -Tag StyleRules {
    $moduleRoot = "..\"

    $files = @(
        Get-ChildItem $moduleRoot\* -Include *.ps1,*.psm1, *.psd1
        Get-ChildItem (Join-Path $moduleRoot 'PSGitLab') -Include *.ps1,*.psm1, *.psd1, *.txt -Recurse
        Get-ChildItem (Join-Path $moduleRoot 'Tests') -Include *.ps1,*.psm1, *.psd1 -Recurse
    )

    It 'Pester source files contain no trailing whitespace' {
        $badLines = @(
            foreach ($file in $files)
            {
                $lines = [System.IO.File]::ReadAllLines($file.FullName)
                $lineCount = $lines.Count

                for ($i = 0; $i -lt $lineCount; $i++)
                {
                    if ($lines[$i] -match '\s+$') {
                        'File: {0}, Line: {1}' -f $file.FullName, ($i + 1)
                    }
                }
            }
        )

        if ($badLines.Count -gt 0)
        {
            throw "The following $($badLines.Count) lines contain trailing whitespace: $([System.Environment]::NewLine)$([System.Environment]::NewLine)$($badLines -join "$([System.Environment]::NewLine)")"
        }
    }
    It 'Spaces are used for indentation in all code files, not tabs' {
        $badLines = @(
            foreach ($file in $files)
            {
                $lines = [System.IO.File]::ReadAllLines($file.FullName)
                $lineCount = $lines.Count

                for ($i = 0; $i -lt $lineCount; $i++)
                {
                    if ($lines[$i] -match '^[  ]*\t|^\t|^\t[  ]*') {
                        'File: {0}, Line: {1}' -f $file.FullName, ($i + 1)
                    }
                }
            }
        )

        if ($badLines.Count -gt 0)
        {
            throw "The following $($badLines.Count) lines start with a tab character: $([System.Environment]::NewLine)$([System.Environment]::NewLine)$($badLines -join "$([System.Environment]::NewLine)")"
        }
    }

    It 'Pester Source Files all end with a newline' {
        $badFiles = @(
            foreach ($file in $files) {
                $string = [System.IO.File]::ReadAllText($file.FullName)
                if ($string.Length -gt 0 -and $string[-1] -ne "`n") {
                    $file.FullName
                }
            }
        )

        if ($badFiles.Count -gt 0) {
            throw "The following files do not end with a newline: $([System.Environment]::NewLine)$([System.Environment]::NewLine)$($badFiles -join "$([System.Environment]::NewLine)")"
        }
    }
}

@ngetchell
Copy link
Owner

I am happy to accept the PR up to commit ffa79be. The additional style rules enforced during the build isn't something I'd like to add.

I've gotten complaints that adding more functions to cover more of the API is overwhelming so I'm going to try to reduce the number of moving parts. I would like to reduce the barrier to entry. This would involve the following:

  • Pester v4 support with the Because parameter to give better feedback
  • No admin rights for build process
  • Much better documentation
  • Faster merging of PR (I think you'd appreciate that one!)

Copy link
Owner

@ngetchell ngetchell left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in feedback. The actual cleanup is great and will definitely be accepted but I'd like the rules to be reverted. See the PR conversation for more details.

@it-praktyk
Copy link
Collaborator Author

@ngetchell, I changed - some time ago - the code as you requested. Can you merge it now?

CC: @X-Guardian, @tdemeester

@ngetchell ngetchell merged commit df7184b into ngetchell:master Jul 31, 2018
@ngetchell
Copy link
Owner

Not sure how I missed this. Thanks for the comment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants