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: SQLAlwaysOnService: Get-TargetResource should return Ensure=Absent #1760

Merged
merged 2 commits into from
Jul 28, 2022

Conversation

jrdbarnes
Copy link
Contributor

@jrdbarnes jrdbarnes commented Jun 23, 2022

Pull Request (PR) description

Remove IsHadrEnabled and return the state in Ensure.
Update Test-TargetResource to look at Ensure instead of IsHadrEnabled.
Update Tests to match.

This Pull Request (PR) fixes the following issues

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).
  • Resource documentation updated in the resource's README.md.
  • Resource parameter descriptions updated in schema.mof.
  • Comment-based help updated, including parameter descriptions.
  • Localization strings updated.
  • Examples updated.
  • Unit tests updated. See DSC Community Testing Guidelines.
  • Integration tests updated (where possible). See DSC Community Testing Guidelines.
  • Code changes adheres to DSC Community Style Guidelines.

This change is Reviewable

…n Ensure=Absent

Remove IsHadrEnabled and return the state in Ensure.
Update Test-TargetResource to look at Ensure instead of IsHadrEnabled.
Update Tests to match.
@codecov
Copy link

codecov bot commented Jun 23, 2022

Codecov Report

Merging #1760 (5f228c9) into main (c71947e) will increase coverage by 0%.
The diff coverage is 100%.

Impacted file tree graph

@@         Coverage Diff          @@
##           main   #1760   +/-   ##
====================================
  Coverage    87%     87%           
====================================
  Files        38      38           
  Lines      6348    6349    +1     
====================================
+ Hits       5574    5575    +1     
  Misses      774     774           
Flag Coverage Δ
unit 87% <100%> (+<1%) ⬆️
Impacted Files Coverage Δ
...DSC_SqlAlwaysOnService/DSC_SqlAlwaysOnService.psm1 100% <100%> (ø)

@jrdbarnes
Copy link
Contributor Author

I'm not entirely sure on the Tests so may need some help. The integration tests were labelled skip, but seem to have run anyway?

@johlju
Copy link
Member

johlju commented Jun 23, 2022

There is a issue with a Windows patch that has broken credential decryption in LCM, so integration tests will fail until Microsoft releases a fix.

@johlju johlju added the needs review The pull request needs a code review. label Jun 23, 2022
@johlju
Copy link
Member

johlju commented Jun 23, 2022

I will review tomorrow or this weekend. 🙂

@johlju
Copy link
Member

johlju commented Jul 19, 2022

I'm waiting for the build workers to get the Windows patch for DSC so the integration tests can run. I will review then.

@johlju
Copy link
Member

johlju commented Jul 27, 2022

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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 r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jrdbarnes)

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 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jrdbarnes)

@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 Jul 28, 2022
@johlju johlju merged commit eb65b8f into dsccommunity:main Jul 28, 2022
@jrdbarnes jrdbarnes deleted the #1759_SQLAlwaysOn_Ensure branch August 2, 2022 12:33
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.

BREAKING CHANGE: SQLAlwaysOnService: Get-TargetResource should return Ensure=Absent
2 participants