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

(MAINT) Schema improvements #203

Merged

Conversation

michaeltlombardi
Copy link
Collaborator

@michaeltlombardi michaeltlombardi commented Sep 24, 2023

PR Summary

This change adds a set of enhancement to the schemas, adding online documentation to the contextual help, new snippets, and reformatting the help.

This change set also includes the following updates to the schemas to reflect the changes for alpha.3:

  1. Replaced the manifestVersion keyword with $schema pointing to the canonical bundled schema URI, per change ManifestVersion to $schema and validate #199.
  2. Replaced set.preTest with set.implementsPretest, per Direct resource set no longer requires test #197.
  3. Replaces args with env for input kind, per add support to pass JSON as env var #198.
  4. Fixes erroneous munging of always-array keywords, like enum and required, to scalar values when the array only contains a single item.

PR Context

This change helps improve the experience for configuration and manifest authors, improving their contextual help and giving them a faster way to add sections and entries to those files.

@mgreenegit
Copy link
Member

Looks like we need to regen again. Frequent changes. Want to put them in the same PR?
https://github.com/michaeltlombardi/DSC/blob/5e10fbbb995d1a7c8ce3f41cf248786603409810/schemas/2023/08/bundled/resource/manifest.json#L14

@michaeltlombardi michaeltlombardi marked this pull request as draft September 27, 2023 19:48
@michaeltlombardi michaeltlombardi marked this pull request as ready for review September 27, 2023 20:09
@michaeltlombardi michaeltlombardi marked this pull request as draft September 27, 2023 20:13
@michaeltlombardi michaeltlombardi marked this pull request as ready for review September 27, 2023 20:57
@michaeltlombardi
Copy link
Collaborator Author

@SteveL-MSFT, @mgreenegit - ended up pulling all the changes for the alpha.3 schema updates into this PR, should be good to go now - also fixed a bunch of errors in the generation process and confirmed the (bundled, at least) schemas are valid now.

${1:metadataName}:
${2:key}: ${3:value}

- label: ' New metadata property (array)'
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a bunch of these have extra whitespace

Suggested change
- label: ' New metadata property (array)'
- label: 'New metadata property (array)'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@SteveL-MSFT - the extra whitespace is intentional, it moves them to the top of the IntelliSense completion list - otherwise they often end up hidden at the bottom, depending on the extensions you have installed.

schemas/src/definitions/inputKind.yaml Outdated Show resolved Hide resolved
Prior to this change, the schema generation relied on hard-coding the full
URL to docs pages, including the version query parameters.

This change updates the build script and configuration for schema generation to
handle replacing the URLs using placeholders.
This change updates the documentation links in the markdown descriptions
for the schemas to:

1. Use the placeholders for more robust linking.
1. Reformat the display so the link is wrapped in horizontal rules.
This change reformats and rewords the Markdown descriptions for
schema enum values. This change only affects the schemas enhanced
for authoring in VS Code.

The standardized format it to include a brief synopsis at the start
of the enum description in italics, then additional information
in a blockquote.
This change reworks the snippets for usability and contextual help.

It:

- Ensures snippets use `body` instead of `bodyText` for non-scalar
  value snippets, so they are correctly inserted instead of relying
  on fragile strings, which might break indentation or syntax.
- Adds many new snippets for faster authoring
- Shows a preview of the snippet insert for the configuration snippets,
  where we can reliably show YAML syntax.
Prior to this change, the markdown description for the semver schema was
used anywhere the value of a property referenced that definition instead
of the defined description on that property.

This change converts the markdownDescription for the definition into a
comment for maintainers, ensuring that more specific documentation can
be shown in the hover context for manifest and configuration authors.
Prior to this change, the schema specified `enums` instead of `enum`, which broke
the validation for that property. This change corrects the spelling and ensures
the validation applies.
This change regenerates the schemas from the updated source
definitions. This ensures the fixes and enhancements are
usable.
This change updates the schemas to reflect the renaming of the
`set.preTest` setting in the resource manifest to the more
readable and semantically correct `set.implementsPretest` to
reflect the change in PowerShell#197.
Prior to this change, the schema generation had a bug that incorrectly
munged the values for always-array keywords, like `enum` and `requires`
to scalar values when the definition was a single-item array.

This change ensures that those values are not incorrectly munged, fixing
hidden bugs in the behavior for those values in the generated schemas.
This change updates the schema definitions to replace the `manifestVersion`
property for resource manifests with the more canonical `$schema` keyword
to reflect the changes from PowerShell#199.
Prior to this change, the config command output schemas referenced
the result data subschemas for the `dsc resource *` commands as
`/<PREFIX>/<VERSION>/results/resource/<COMMAND>.yaml` instead of
`/<PREFIX>/<VERSION>/outputs/resource/<COMMAND>.yaml`, which is
their actual canonical URI.

This change fixes the error, ensuring the schemas validate correctly.
This change updates the schemas to replace the unimplemented `args` option
for command input in the resource manifest with the `env` option. It also
updates the schema and documentation to indicate that when the `input` option
is not defined for a command in a resource manifest, DSC does not send the
resource any input for that command.

This update is made to reflect the changes from PowerShell#198.
This change standardizes formatting for the default snippets
definitions in the source schemas and ensures that every
snippet label starts with a leading space - this puts them
at the top of the IntelliSense list for completions when
they are available.
Prior to this change, the enum documentation for the enhanced schema
didn't clarify how the environment variables are named or their values
passed.

This change clarifies both, making it easier for manifest authors to
determine whether the input option is right for them and how they should
expect to handle the input in their resource without consulting the
online documentation.
Prior to this change, the schema docs erroneously noted that
when `input` is not defined in a resource manifest for the
`set` and `test` definitions that DSC doesn't send any input.

This is _technically_ true, but the `input` property is included
in the list of `required` properties for `set` and `test`, unlike
`get`.

This change removes the misleading sentence, since those properties
can't be undefined in a valid manifest.
@SteveL-MSFT SteveL-MSFT added this pull request to the merge queue Sep 29, 2023
Merged via the queue into PowerShell:main with commit 75ae582 Sep 29, 2023
4 checks passed
@michaeltlombardi michaeltlombardi deleted the maint/main/schema-improvements branch September 30, 2023 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants