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

BREAKING CHANGE: Update to new CI #654

Merged
merged 36 commits into from Dec 29, 2019
Merged

Conversation

PlagueHO
Copy link
Member

@PlagueHO PlagueHO commented Dec 28, 2019

Pull Request (PR) description

This PR updates to the new CI process.

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry under the Unreleased section of the change log in
    CHANGELOG.md. Entry should say what was changed, and how that affects
    users (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as
    appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See
    DSC Resource Testing Guidelines.
  • Integration tests added/updated (where possible). See
    DSC Resource Testing Guidelines.
  • New/changed code adheres to
    DSC Resource Style Guidelines and Best Practices

This change is Reviewable

@PlagueHO
Copy link
Member Author

@mhendric - once this PR is merged, the repo will be in a temporary state that will not be releasable until I add the AzDO pipeline. I'll be removing the AppVeyor, CLA and other processes at the same time. So please don't merge anything until this PR is through.

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.

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


azure-pipelines.yml, line 97 at r1 (raw file):

pathToSources: '$(Build.SourcesDirectory)/output/$(DscBuildVariable.RepositoryName)

Missing the powershell task that sets DscBuildVariable (see the "Steps")


.github/CONTRIBUTING.md, line 1 at r1 (raw file):

If you'd like to contribute to this project, please review the [Contribution Guidelines](https://github.com/PowerShell/DscResources/blob/master/CONTRIBUTING.md).

Wrong URL here.


.github/PULL_REQUEST_TEMPLATE.md, line 51 at r1 (raw file):

testing-guideline

URL: testing-guidelines (should end with 's')


.github/PULL_REQUEST_TEMPLATE.md, line 52 at r1 (raw file):

testing-guideline

URL: testing-guidelines (should end with 's')


source/xPSDesiredStateConfiguration.psd1, line 46 at r1 (raw file):

Quoted 6 lines of code…
    DscResourcesToExport  = @(
        'xArchive', 'xDSCWebService', 'xEnvironment','xGroup','xMsiPackage',
        'xPackage','xPSSessionConfiguration','xRegistry','xRemoteFile',
        'xScript','xService','xUser','xWindowsFeature','xWindowsOptionalFeature',
        'xWindowsPackageCab','xWindowsProcess'
    )

Information: I will add this to the "Steps" too!


source/DSCResources/DSC_xPSSessionConfiguration/DSC_xPSSessionConfiguration.schema.mof, line 1 at r1 (raw file):

xPSEndpoint

Should this say xPsSessionConfiguration? 🤔


source/en-US/about_xPSDesiredStateConfiguration.help.txt, line 5 at r1 (raw file):

configuring WS-Man.

Sounds wrong. 😉 Same for the long description below.


tests/Integration/DSC_xDSCWebService.Integration.tests.ps1, line 24 at r1 (raw file):

-ErrorAction:Stop

Should be ErrorAction 'Stop'? Did not know it was possible to write it using colon 🤔


tests/Unit/DSC_xRemoteFile.Tests.ps1, line 482 at r1 (raw file):

$False

$false (lower-case). Throughout.


tests/Unit/DSC_xRemoteFile.Tests.ps1, line 500 at r1 (raw file):

$True

$true (lower-case). Throughout.

@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 Dec 28, 2019
Copy link
Member Author

@PlagueHO PlagueHO 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: 275 of 281 files reviewed, 9 unresolved discussions (waiting on @johlju)


azure-pipelines.yml, line 97 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
pathToSources: '$(Build.SourcesDirectory)/output/$(DscBuildVariable.RepositoryName)

Missing the powershell task that sets DscBuildVariable (see the "Steps")

Done.


.github/CONTRIBUTING.md, line 1 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Wrong URL here.

Done.


.github/PULL_REQUEST_TEMPLATE.md, line 51 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
testing-guideline

URL: testing-guidelines (should end with 's')

Done.


.github/PULL_REQUEST_TEMPLATE.md, line 52 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
testing-guideline

URL: testing-guidelines (should end with 's')

Done.


source/DSCResources/DSC_xPSSessionConfiguration/DSC_xPSSessionConfiguration.schema.mof, line 1 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
xPSEndpoint

Should this say xPsSessionConfiguration? 🤔

This is correct, the FriendlyName is supposed to be xPSEndpoint. At some point it might be worth renaming the resource to xPSSessionConfiguration but probably a future change. I did mislabel it in the manifest though so I've corrected that.


source/en-US/about_xPSDesiredStateConfiguration.help.txt, line 5 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
configuring WS-Man.

Sounds wrong. 😉 Same for the long description below.

Done.


tests/Integration/DSC_xDSCWebService.Integration.tests.ps1, line 24 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
-ErrorAction:Stop

Should be ErrorAction 'Stop'? Did not know it was possible to write it using colon 🤔

Done.


tests/Unit/DSC_xRemoteFile.Tests.ps1, line 482 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
$False

$false (lower-case). Throughout.

Done.


tests/Unit/DSC_xRemoteFile.Tests.ps1, line 500 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
$True

$true (lower-case). Throughout.

Done.

@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 Dec 29, 2019
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 16 of 16 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


tests/Integration/DSC_xDSCWebService.Integration.tests.ps1, line 32 at r2 (raw file):

if ($env:CI -eq $false)

Non-blocking: There are no property in Azure Pipelines called CI (I noticed that in SqlServerDsc as I need to add it to azure-pipelines.yml as a variable). This will always run, since the tests are passing this is a non-issue. But might be worth adding an issue about cleaning this code up? 🤔

@johlju johlju added ready for merge The pull request was approved by the community and is ready to be merged by a maintainer. and removed needs review The pull request needs a code review. labels Dec 29, 2019
@PlagueHO PlagueHO merged commit c65d112 into dsccommunity:dev Dec 29, 2019
@PlagueHO PlagueHO deleted the Issue-650 branch December 29, 2019 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for merge The pull request was approved by the community and is ready to be merged by a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update module to new CI and release process
2 participants