Conversation
kejadlen
force-pushed
the
alpha/push-tmorzztppuux
branch
3 times, most recently
from
6d4a36e to
dafdf31
Compare
rubberduck203
left a comment
rubberduck203
left a comment
There was a problem hiding this comment.
We should test this against our internal library of doctor groups/known errors sooner than later to ensure everything still parses correctly.
| "DoctorCheckSpec": { | ||
| "description": "What needs to be checked before the action will run. `paths` will be checked first, then `commands`. If a `path` matches no files or the matching files have changed, the `command` will run.", | ||
| "description": "What needs to be checked before the action will run. `paths` will be checked first, then\n`commands`. If a `path` matches no files or the matching files have changed, the `command` will run.", |
There was a problem hiding this comment.
Weird new line break was introduced.
There was a problem hiding this comment.
Seems like a change in schemars: GREsau/schemars#120
I had Claude write a transform and added that to this PR, but not sure it's worth the extra step to collapse the descriptions or not.
There was a problem hiding this comment.
Got it. Possibly not worth the effort. It just caught my eye.
There was a problem hiding this comment.
I'll go ahead and revert the transformer then - agree that probably it's not worth the complexity to de-newline the docs.
| }, | ||
| "paths": { | ||
| "description": "A list of globs to check for changes. When the glob matches a new file, or the contents of the file change, the check will require a fix.\n\nRelative paths are relative to the scope config directory containing the config file.\n\nShared configs can use the template string `{{ working_dir }}` to access the working directory.", | ||
| "default": null, | ||
| "description": "A list of globs to check for changes. When the glob matches a new file, or the contents\nof the file change, the check will require a fix.\n\nRelative paths are relative to the scope config directory containing the config file.\n\nShared configs can use the template string `{{ working_dir }}` to access the working\ndirectory.", |
There was a problem hiding this comment.
Here as well. Some kind of behavior change in the schema generator?
| }, | ||
| "DoctorInclude": { | ||
| "description": "Configure how a groups will be used when determining the task graph.", | ||
| "oneOf": [ | ||
| { | ||
| "description": "Default option, the group will be included by default when determining which groups should run.", | ||
| "description": "Default option, the group will be included by default when determining which groups should\nrun.", |
There was a problem hiding this comment.
It's littered with these changes. Do we need to update the doc comments they come from?
| "enum": [ | ||
| "when-required" | ||
| ] | ||
| "const": "when-required" |
There was a problem hiding this comment.
This itself is not a breaking change, but it looks like the intent was for it to be extendable.
Would later changing it back to an enum and extending it be a breaking change?
There was a problem hiding this comment.
I think this is actually the same? I believe enum in JSON Schema is similarly non-extendable?
There was a problem hiding this comment.
Yes, as far as “you can’t add any value you want, so an enum with one member is constant”.
https://json-schema.org/understanding-json-schema/reference/enum
I’m a little rusty on my json schema and trying to think through if moving back from constant to enum later would be compatible. I think it’s fine.
There was a problem hiding this comment.
Also this is a child of an anyOf field, which is the actual enum, presumably exploded out into anyOf instead of enum for the descriptions.
kejadlen
force-pushed
the
alpha/push-tmorzztppuux
branch
8 times, most recently
from
f77f60c to
f404308
Compare
Assisted-by: Claude Opus 4.5 via Claude Code
kejadlen
force-pushed
the
alpha/push-tmorzztppuux
branch
from
f404308 to
c669d09
Compare
3 participants
Upgrades schemars from 0.8 to 1.2 and adapts to API changes.
API changes:
schemars::genmodule renamed toschemars::generateoption_nullablesetting removed—schemars 1.x generatestype: ["string", "null"]for Option types nativelySchema compatibility fixes:
skip_serializing_ifto#[schemars(skip)]fields that were serializing to null but falling underadditionalProperties: { type: "string" }deny_unknown_fieldsfromReportDestinationTemplateswhich conflicted with its flattenedBTreeMapNormalizeDescriptionstransform to collapse single line breaks into spaces while preserving paragraph breaks (\n\n). See Handling of newline in RustDoc GREsau/schemars#120Test fix:
directories.rsto prevent flaky failures from test parallelismAssisted-by: Claude Opus 4.5 via Claude Code