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

WebConfigPropertyCollection - allow different property collection key types to be added, IisModule / IisMimeTypeMapping - set Ensure as default #640

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Borgquite
Copy link
Contributor

@Borgquite Borgquite commented Oct 15, 2024

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

None

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 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 Community Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

Copy link

codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88%. Comparing base (da0cd60) to head (808f7cf).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@         Coverage Diff         @@
##           main   #640   +/-   ##
===================================
  Coverage    88%    88%           
===================================
  Files        15     15           
  Lines      1952   1952           
===================================
  Hits       1726   1726           
  Misses      226    226           
Files with missing lines Coverage Δ
...DSC_IisMimeTypeMapping/DSC_IisMimeTypeMapping.psm1 100% <ø> (ø)
...urce/DSCResources/DSC_IisModule/DSC_IisModule.psm1 0% <ø> (ø)
...rtyCollection/DSC_WebConfigPropertyCollection.psm1 98% <ø> (ø)

@johlju johlju added the needs review The pull request needs a code review. label Oct 16, 2024
Copy link

@dan-hughes dan-hughes left a comment

Choose a reason for hiding this comment

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

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Borgquite)

a discussion (no related file):
Can you update the examples and integration tests to cover the default behaviour?

For the integration tests do not change them all but some to prove that if Ensure is supplied it works and also when not.


Copy link
Contributor Author

@Borgquite Borgquite 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: 7 of 9 files reviewed, 1 unresolved discussion (waiting on @dan-hughes)

a discussion (no related file):

Previously, dan-hughes (Daniel Hughes) wrote…

Can you update the examples and integration tests to cover the default behaviour?

For the integration tests do not change them all but some to prove that if Ensure is supplied it works and also when not.

Done.


Copy link
Contributor Author

@Borgquite Borgquite 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: 7 of 9 files reviewed, 1 unresolved discussion (waiting on @dan-hughes)

a discussion (no related file):

Previously, Borgquite (Chris H) wrote…

Done.

There are no examples or integration tests for IISModule so can't really do them :( but done for IisMimeTypeMapping.


Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review The pull request needs a code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants