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

VhdFileDirectory - improvements and fixes to Content files #179

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DmitryMashkov-DM
Copy link

@DmitryMashkov-DM DmitryMashkov-DM commented Jan 7, 2020

Pull Request (PR) description

  1. Added ContentEncoding property to properly support Content files (otherwise they are always written as Unicode) which corrupts some content types like UTF-8 XMLs.
  2. Content files are not tested with ItemHasChanged (this didn't work anyway).
  3. Fixed GetItemToCopy where an undefined DefaultValues array was used instead of DesiredProperties, resulting in a dead code, impacting properties like Force.
  4. After Update versions and README for release #3 is fixed and the code is finally execuded, it would crash if a file is a Content (without SourcePath).
  5. In SetVHDFile , Copy-Item's ErrorAction is set to Stop. Otherwise, the resource would simply ignore all errors and produce invalid configuration.

This Pull Request (PR) fixes the following issues

None


This change is Reviewable

…otherwise they are always written as Unicode) which corrupts some content types like UTF-8 XMLs.

2. Content files are not tested with ItemHasChanged (this didn't work anyway).
3. Fixed GetItemToCopy where an undefined DefaultValues array was used instead of DesiredProperties, resulting in a dead code, impacting properties like Force.
4. After dsccommunity#3 is fixed and the code is finally execuded, it would crash if a file is a Content (without SourcePath).
5. In SetVHDFile , Copy-Item's ErrorAction is set to Stop. Otherwise, the resource would simply ignore all errors and produce invalid configuration.
@codecov
Copy link

codecov bot commented Jan 7, 2020

Codecov Report

Merging #179 into dev will decrease coverage by <1%.
The diff coverage is 0%.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev   #179   +/-   ##
===================================
- Coverage    73%    73%   -1%     
===================================
  Files        11     11           
  Lines      1401   1402    +1     
===================================
  Hits       1035   1035           
- Misses      366    367    +1

@aromano2
Copy link

Hi Dmitry, can you please open an issue for this feature so that we have more context? Also, could you add appropriate tests for the new functionality please? Thanks for the contribution!

@aromano2 aromano2 added the waiting for author response The pull request is waiting for the author to respond to comments in the pull request. label Jan 30, 2020
@johlju johlju changed the base branch from dev to master March 19, 2020 17:00
@johlju
Copy link
Member

johlju commented Jun 8, 2022

We have renamed the resource, removing 'x', so please rebase this PR.

@johlju johlju changed the title xVhdFileDirectory - improvements and fixes to Content files VhdFileDirectory - improvements and fixes to Content files Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for author response The pull request is waiting for the author to respond to comments in the pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants