Make bundle/upgrade print YAML if the input was YAML - #888
Conversation
🤖 Augment PR SummarySummary: Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
There was a problem hiding this comment.
Review completed. 2 suggestions posted.
Items Reviewed
- ✅ Review PR #888 changes and post one GitHub review with an approximately 12-line summary.
Comment augment review to trigger a new review at any time.
| : read_file(schema_path)}; | ||
| auto parsed_schema{schema_from_stdin | ||
| ? read_from_stdin(nullptr, InputFormatting::Preserve) | ||
| : read_file(schema_path, InputFormatting::Preserve)}; |
There was a problem hiding this comment.
src/command_bundle.cc:45: In preserve mode, read_yaml_file flags subsequent YAML documents in parsed_schema.multidocument, but this command never inspects it; a multi-document YAML file therefore bundles only the first document and silently drops the rest from stdout. read_from_stdin rejects this case, so file and stdin inputs now behave inconsistently. Other locations where this applies: src/command_upgrade.cc:182.
Severity: medium
Other Locations
src/command_upgrade.cc:182
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| sourcemeta::core::prettify(schema, std::cout); | ||
| std::cout << "\n"; | ||
| sourcemeta::jsonschema::write_schema(schema, std::cout, | ||
| parse_optional_indentation(options), |
There was a problem hiding this comment.
src/command_bundle.cc:123: parse_optional_indentation runs only after configuration loading, input parsing, reference resolution, and bundling, so malformed --indentation can be masked by an input/schema error (and can trigger remote resolution) instead of returning the intended invalid-CLI-arguments error. Other locations where this applies: src/command_upgrade.cc:202.
Severity: low
Other Locations
src/command_upgrade.cc:202
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
2 issues found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="test/bundle/pass_yaml_indentation.clitest">
<violation number="1" location="test/bundle/pass_yaml_indentation.clitest:16">
P3: The PR title covers both `bundle` and `upgrade`, but this test file only exercises `bundle`. If the upgrade path shares the YAML-output/`--indentation` behavior, add an equivalent run for `upgrade schema.yaml --resolve remote.yaml` (and the `--indentation 2` variant) so the new behavior is asserted on both commands.</violation>
</file>
<file name="src/command_bundle.cc">
<violation number="1" location="src/command_bundle.cc:123">
P3: Parse `--indentation` before configuration loading and schema processing in both commands; late validation can be masked by input errors or trigger reference resolution for an invalid CLI value.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| type: string | ||
| EOF | ||
|
|
||
| RUN bundle schema.yaml --resolve remote.yaml STDIN /dev/null IN . INTO result_0.txt EXPECTING 0 |
There was a problem hiding this comment.
P3: The PR title covers both bundle and upgrade, but this test file only exercises bundle. If the upgrade path shares the YAML-output/--indentation behavior, add an equivalent run for upgrade schema.yaml --resolve remote.yaml (and the --indentation 2 variant) so the new behavior is asserted on both commands.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/bundle/pass_yaml_indentation.clitest, line 16:
<comment>The PR title covers both `bundle` and `upgrade`, but this test file only exercises `bundle`. If the upgrade path shares the YAML-output/`--indentation` behavior, add an equivalent run for `upgrade schema.yaml --resolve remote.yaml` (and the `--indentation 2` variant) so the new behavior is asserted on both commands.</comment>
<file context>
@@ -0,0 +1,50 @@
+type: string
+EOF
+
+RUN bundle schema.yaml --resolve remote.yaml STDIN /dev/null IN . INTO result_0.txt EXPECTING 0
+
+WRITE expected_0.txt UNTIL EOF
</file context>
| sourcemeta::core::prettify(schema, std::cout); | ||
| std::cout << "\n"; | ||
| sourcemeta::jsonschema::write_schema(schema, std::cout, | ||
| parse_optional_indentation(options), |
There was a problem hiding this comment.
P3: Parse --indentation before configuration loading and schema processing in both commands; late validation can be masked by input errors or trigger reference resolution for an invalid CLI value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/command_bundle.cc, line 123:
<comment>Parse `--indentation` before configuration loading and schema processing in both commands; late validation can be masked by input errors or trigger reference resolution for an invalid CLI value.</comment>
<file context>
@@ -118,6 +119,7 @@ auto sourcemeta::jsonschema::bundle(const sourcemeta::core::Options &options)
- sourcemeta::core::prettify(schema, std::cout);
- std::cout << "\n";
+ sourcemeta::jsonschema::write_schema(schema, std::cout,
+ parse_optional_indentation(options),
+ parsed_schema.roundtrip);
}
</file context>
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
ce37afb to
67b523b
Compare
|
@cubic-dev-ai review |
@jviotti I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 15 files
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
Signed-off-by: Juan Cruz Viotti jv@jviotti.com