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

[Invoke-Git] Return Object & Change to Public Function #80

Merged
merged 10 commits into from
Jun 26, 2021

Conversation

phbits
Copy link
Contributor

@phbits phbits commented Jun 10, 2021

Pull Request

Pull Request (PR) description

  • Invoke-Git
    • Now a public function since Tasks can only use public functions.
    • Returns an object allowing caller to take appropriate action.
    • Updated tests.

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Documentation added/updated in README.md.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated.
  • Integration tests added/updated (where possible).
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

@codecov
Copy link

codecov bot commented Jun 10, 2021

Codecov Report

Merging #80 (4f7a2e8) into main (27e84f3) will increase coverage by 0%.
The diff coverage is 100%.

Impacted file tree graph

@@        Coverage Diff         @@
##           main   #80   +/-   ##
==================================
  Coverage    97%   97%           
==================================
  Files        38    38           
  Lines       865   877   +12     
==================================
+ Hits        846   859   +13     
+ Misses       19    18    -1     

@johlju johlju added the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Jun 16, 2021
@johlju
Copy link
Member

johlju commented Jun 19, 2021

It seems the changes did not work as expected, it failed to publish to the Wiki. See error https://dev.azure.com/dsccommunity/DscResource.DocGenerator/_build/results?buildId=4801&view=logs&j=5d0a9c4e-8741-5118-af45-406a30fe661c&t=bda0b158-a237-516f-c1d0-31339587c5b5&l=146

2021-06-19T15:25:33.0578840Z Publishing Wiki content.
2021-06-19T15:25:33.2106611Z ERROR: Exception calling "Start" with "0" argument(s): "No such file or directory"
2021-06-19T15:25:33.2107271Z At /home/vsts/work/1/s/output/DscResource.DocGenerator/0.9.0/DscResource.DocGenerator.psm1:1402 char:13
2021-06-19T15:25:33.2108430Z +         if ($process.Start() -eq $true)
2021-06-19T15:25:33.2108871Z +             ~~~~~~~~~~~~~~~~~~~~~~~~~~
2021-06-19T15:25:33.2151472Z At /home/vsts/work/1/s/source/tasks/Publish_GitHub_Wiki_Content.build.ps1:102 char:1

@johlju
Copy link
Member

johlju commented Jun 19, 2021

Maybe this PR is needed to complete everything? 🙂

@johlju
Copy link
Member

johlju commented Jun 20, 2021

@phbits I will review this once you rebased this PR. 🙂

@johlju johlju self-requested a review June 20, 2021 13:06
@phbits phbits force-pushed the InvokeGitRewrite branch from 1db68b7 to e298ac3 Compare June 21, 2021 17:33
@phbits
Copy link
Contributor Author

phbits commented Jun 22, 2021

@phbits I will review this once you rebased this PR. 🙂

I did the rebase and had to do a force push. After manually resolving conflicts and fixing typos, this PR looks ready.

@phbits phbits changed the title [Invoke-Git] Update to use System.Diagnostics.Process [Invoke-Git] Return Object & Change to Public Function Jun 23, 2021
@johlju johlju added needs review The pull request needs a code review. and removed waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. labels Jun 25, 2021
Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

Awesome work! Just a few minor comments. 😃

Reviewed 9 of 9 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @phbits)


source/Public/Publish-WikiContent.ps1, line 115 at r1 (raw file):

Quoted 6 lines of code…
            Write-Verbose -Message $script:localizedData.ConfigGlobalGitMessage
            if ($PSBoundParameters.ContainsKey('GlobalCoreAutoCrLf'))
            {
                $null = Invoke-Git -WorkingDirectory $tempPath.FullName `
                            -Arguments @( 'config', '--local', 'core.autocrlf', $GlobalCoreAutoCrLf )
            }

This must be run before cloning, since it tells git how to treat linebreaks during clone.


tests/unit/public/Publish-WikiContent.Tests.ps1, line 139 at r1 (raw file):

 Context 'When publishing Wiki content' {

Can we add a new line before this line?


tests/unit/tasks/Publish_GitHub_Wiki_Content.build.Tests.ps1, line 39 at r1 (raw file):

Quoted 5 lines of code…
        } -ParameterFilter {
                $Arguments[0] -eq 'remote' -and
                $Arguments[1] -eq 'get-url' -and
                $Arguments[2] -eq 'origin'
            }

Indentation looks wrong here.


tests/unit/tasks/Publish_GitHub_Wiki_Content.build.Tests.ps1, line 116 at r1 (raw file):

Quoted 4 lines of code…
                    $Arguments[0] -eq 'remote' -and
                    $Arguments[1] -eq 'get-url' -and
                    $Arguments[2] -eq 'origin'
                }

Indentation looks wrong here.

@johlju johlju added waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. and removed needs review The pull request needs a code review. labels Jun 25, 2021
@johlju
Copy link
Member

johlju commented Jun 25, 2021

@phbits you need to rebase again since another PR was merged. 🙂

@phbits phbits force-pushed the InvokeGitRewrite branch from 29a65ac to 4c83c25 Compare June 25, 2021 22:38
Copy link
Contributor Author

@phbits phbits left a comment

Choose a reason for hiding this comment

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

Reviewable status: 4 of 9 files reviewed, 4 unresolved discussions (waiting on @johlju)


source/Public/Publish-WikiContent.ps1, line 115 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
            Write-Verbose -Message $script:localizedData.ConfigGlobalGitMessage
            if ($PSBoundParameters.ContainsKey('GlobalCoreAutoCrLf'))
            {
                $null = Invoke-Git -WorkingDirectory $tempPath.FullName `
                            -Arguments @( 'config', '--local', 'core.autocrlf', $GlobalCoreAutoCrLf )
            }

This must be run before cloning, since it tells git how to treat linebreaks during clone.

Done.


tests/unit/public/Publish-WikiContent.Tests.ps1, line 139 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
 Context 'When publishing Wiki content' {

Can we add a new line before this line?

Done.


tests/unit/tasks/Publish_GitHub_Wiki_Content.build.Tests.ps1, line 39 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
        } -ParameterFilter {
                $Arguments[0] -eq 'remote' -and
                $Arguments[1] -eq 'get-url' -and
                $Arguments[2] -eq 'origin'
            }

Indentation looks wrong here.

Done.


tests/unit/tasks/Publish_GitHub_Wiki_Content.build.Tests.ps1, line 116 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
                    $Arguments[0] -eq 'remote' -and
                    $Arguments[1] -eq 'get-url' -and
                    $Arguments[2] -eq 'origin'
                }

Indentation looks wrong here.

Done.

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 5 of 5 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @phbits)

@johlju johlju removed the waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. label Jun 26, 2021
@johlju johlju merged commit c75f492 into dsccommunity:main Jun 26, 2021
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