Sampler icon indicating copy to clipboard operation
Sampler copied to clipboard

Init migrate PlatyPS to Microsoft.PowerShell.PlatyPS

Open Gijsreyn opened this issue 1 year ago • 8 comments

Pull Request

Pull Request (PR) description

Initialize migrating from PlatyPS to Microsoft.PowerShell.PlatyPS. Relates to #497

Deprecated

  • PlatyPS module

Task list

  • [x] The PR represents a single logical change. i.e. Cosmetic updates should go in different PRs.
  • [ ] Added an entry under the Unreleased section of in the CHANGELOG.md as per format.
  • [ ] Local clean build passes without issue or fail tests (build.ps1 -ResolveDependency).
  • [ ] Documentation added/updated in README.md.
  • [ ] Comment-based help added/updated.
  • [ ] Localization strings added/updated in all localization files as appropriate.
  • [ ] 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

Gijsreyn avatar Dec 01 '24 14:12 Gijsreyn

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 81%. Comparing base (99707fb) to head (cea87f1).

Files with missing lines Patch % Lines
.build/tasks/generateHelp.PlatyPS.build.ps1 0% 1 Missing :warning:
Additional details and impacted files

Impacted file tree graph

@@         Coverage Diff         @@
##           main   #501   +/-   ##
===================================
  Coverage    81%    81%           
===================================
  Files        44     44           
  Lines      2337   2335    -2     
===================================
  Hits       1912   1912           
+ Misses      425    423    -2     
Files with missing lines Coverage Δ
.build/tasks/generateHelp.PlatyPS.build.ps1 0% <0%> (ø)

codecov[bot] avatar Dec 01 '24 14:12 codecov[bot]

@Gijsreyn are you still working on this? I waiting for the tests to pass.

johlju avatar Feb 16 '25 18:02 johlju

I thought the changelog only needed to be updated so all tests would pass, but I cannot seem why there are suddenly so many failures. I'll close it down to not break stuff @johlju.

Gijsreyn avatar Feb 17 '25 18:02 Gijsreyn

It looks like the errors were not the result of this PR but a change in DscResource.Test. You can reopen this if you like and I will fix the issue in a separate PR.

johlju avatar Feb 17 '25 18:02 johlju

It looks like the errors were not the result of this PR but a change in DscResource.Test. You can reopen this if you like and I will fix the issue in a separate PR.

Done! I was in shock in the failing tests... Let me know on the fix and then we can try again :)

Gijsreyn avatar Feb 18 '25 05:02 Gijsreyn

There are seemingly a bug in ModuleBuilder that prevents the integration tests in this repo from working fully on PS5.1, see https://github.com/PoshCode/ModuleBuilder/issues/135. That needs to be fixed first before I can merge a fix to solve the issue with DscResource.Test in PR https://github.com/gaelcolas/Sampler/pull/514. 🙂

johlju avatar Feb 18 '25 11:02 johlju

I have finally merged a workaround for the problem with ModuleBuilder. Please resolve the conflicts in this PR (merge in main) and the tests should work again.

johlju avatar Feb 28 '25 10:02 johlju

Very much appreciated @johlju. Great job on the merge. I've just resolved the conflicts. Should be good to go.

Gijsreyn avatar Feb 28 '25 10:02 Gijsreyn

Hey @johlju, do you perhaps have time to pull this one in?

Gijsreyn avatar May 01 '25 07:05 Gijsreyn

I will try to get some time to look at this. Do you know if there are any negative consequences of this change in generated docs output?

johlju avatar May 03 '25 06:05 johlju

@johlju Thanks for the response. It shouldn't have any negative consequences. Instead, it should help migrate to the newer formats, e.g., extended parameter settings are documented.

Gijsreyn avatar May 03 '25 08:05 Gijsreyn

This is what I found doing a quick comparison:

  • Help file is created in output/help/0.0.1/en-US/SqlServerDsc/SqlServerDsc-Help.xml instead of output/help/0.0.1/en-US/SqlServerDsc-help.xml
  • Markdown is created in output/docs/SqlServerDsc instead of output/docs
  • With the suggested the task generates correct help for mandatory parameters, which current task does not.
  • The suggested change does not generate code blocks for the examples, which the current task does.
  • The YAML Front Matter in markdown no longer contains tag online version and schema.
  • It adds the header ### __AllParameterSets above the syntax in certain commands (even though they are not using parameter sets).
  • The suggested change adds the header Alias even if the command does not contain aliases, and instead adds the text {{Insert list of aliases}} under the header
  • For each parameter it adds more properties (which at first glance looks okay) and uses full type name.
  • Header INPUTS are added with sub header using text in the comment-based help, but also adds the text. {{ Fill in the Description }}
  • Header OUTPUTS are added with sub header using text in the comment-based help, but also adds the text. {{ Fill in the Description }}
  • Header RELATED LINKS with the text {{ Fill in the related links here }} unless .LINK is used in comment-based help, then the link is outputed.
  • Common parameters like -Confirm and -WhatIf have text like {{ Fill WhatIf Description }}.

This probably have consequences for those using this task, if it can't be resolved one way or the other. Should probably be documented.

johlju avatar May 03 '25 08:05 johlju

Do you want me to take a look at it, or want me to document it? If so, on the documentation part, do you have any suggestions?

Gijsreyn avatar May 03 '25 08:05 Gijsreyn

The best would be to document what a comment-based help must look like to get the best output, and what the user need to do after the docs have been generated. This would be best documented in the README.md under Tasks.

Then in the changelog document all the breaking changes like the paths that differ.

johlju avatar May 03 '25 08:05 johlju

Gotcha, thanks a lot again. I'll put it on my TODO list and let you know when its done.

Gijsreyn avatar May 03 '25 08:05 Gijsreyn