ignore diagramfiles validation #2
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
Reviewer's GuideThis PR updates the build-and-publish GitHub Actions workflow to temporarily exclude *.diagram.json files from vNext validation by renaming them before the validation step, restoring them afterwards, and improving error reporting for validation failures. Sequence diagram for updated CI validation and diagram file handlingsequenceDiagram
participant Dev as Developer
participant GitHubActions as GitHub_Actions_Workflow
participant Runner as Workflow_Runner
participant FS as Repository_Filesystem
participant Validator as Vnext_Validator
Dev->>GitHubActions: Push changes / open PR
GitHubActions->>Runner: Start build_and_publish workflow
Runner->>FS: Find core/*.diagram.json
FS-->>Runner: List of diagram files (if any)
alt Diagram files found
Runner->>FS: Rename each *.diagram.json to *.diagram.json.skip
else No diagram files
Runner-->>Runner: Skip renaming
end
Runner->>Validator: vnext validate --resolve-refs
alt Validation succeeds
Validator-->>Runner: Exit code 0
Runner->>FS: Find *.diagram.json.skip and restore to *.diagram.json
Runner-->>GitHubActions: Continue workflow (validation success)
else Validation fails
Validator-->>Runner: Exit code != 0
Runner->>Runner: Store VALIDATION_EXIT_CODE
Runner->>FS: Find *.diagram.json.skip and restore to *.diagram.json
Runner->>Runner: Print detailed validation error hints
Runner-->>GitHubActions: Exit 1 (fail job)
end
Flow diagram for updated vNext validation step in CI workflowflowchart TD
subgraph job_validate_vnext["Job validate_vnext in build_and_publish workflow"]
A_start["Start validation step"] --> B_find_diagrams
B_find_diagrams["Find *.diagram.json files under core"] --> C_has_diagrams{Any diagram files found?}
C_has_diagrams -- Yes --> D_rename_loop["For each diagram file: mv file to file.skip"]
D_rename_loop --> E_run_validation
C_has_diagrams -- No --> E_run_validation["Run vnext validate --resolve-refs"]
E_run_validation --> F_validation_ok{Validation successful?}
F_validation_ok -- Yes --> G_restore_files["Find and restore *.diagram.json.skip back to *.diagram.json"]
G_restore_files --> H_success["Echo success message and continue workflow"]
F_validation_ok -- No --> I_capture_exit["Capture validation exit code"]
I_capture_exit --> G_restore_files
G_restore_files --> J_error_check{Validation exit code nonzero?}
J_error_check -- Yes --> K_print_errors["Print detailed validation error guidance"]
K_print_errors --> L_fail_step["Exit 1 (fail job)"]
J_error_check -- No --> H_success
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The use of
echo "$DIAGRAM_FILES" | while read file; do ...will split on whitespace and may break for diagram file paths containing spaces; consider usingfind ... -print0withwhile IFS= read -r -d '' fileorwhile IFS= read -r file; do ... done <<< "$DIAGRAM_FILES"to handle arbitrary file names safely. - Relying on temporary renames (
mv "$file" "${file}.skip") across the entirecoretree is a bit brittle (e.g., if a later command exits before the restore loop runs); if possible, limit the scope of the rename to the exact directories validated or adjust the validation command to ignore the diagram files instead of mutating the working tree.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The use of `echo "$DIAGRAM_FILES" | while read file; do ...` will split on whitespace and may break for diagram file paths containing spaces; consider using `find ... -print0` with `while IFS= read -r -d '' file` or `while IFS= read -r file; do ... done <<< "$DIAGRAM_FILES"` to handle arbitrary file names safely.
- Relying on temporary renames (`mv "$file" "${file}.skip"`) across the entire `core` tree is a bit brittle (e.g., if a later command exits before the restore loop runs); if possible, limit the scope of the rename to the exact directories validated or adjust the validation command to ignore the diagram files instead of mutating the working tree.
## Individual Comments
### Comment 1
<location> `.github/workflows/build-and-publish.yml:96-108` </location>
<code_context>
+ # Temporarily rename diagram files to exclude them from validation
+ echo "📋 Excluding diagram files from validation..."
+ DIAGRAM_FILES=$(find core -name "*.diagram.json" -type f 2>/dev/null || true)
+ if [ -n "$DIAGRAM_FILES" ]; then
+ echo "$DIAGRAM_FILES" | while read file; do
+ if [ -f "$file" ]; then
+ mv "$file" "${file}.skip"
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The `echo ... | while read` pattern is brittle for filenames with spaces and can be simplified.
`echo "$DIAGRAM_FILES" | while read file` will split on spaces and special characters, so any diagram path containing them may be processed incorrectly. You can avoid this by iterating directly over `find`’s output and preserving paths:
```sh
find core -name "*.diagram.json" -type f 2>/dev/null | while IFS= read -r file; do
mv "$file" "${file}.skip"
echo " Excluded: $file"
done
```
```suggestion
# Temporarily rename diagram files to exclude them from validation
echo "📋 Excluding diagram files from validation..."
DIAGRAM_FILES=$(find core -name "*.diagram.json" -type f 2>/dev/null || true)
if [ -n "$DIAGRAM_FILES" ]; then
find core -name "*.diagram.json" -type f 2>/dev/null | while IFS= read -r file; do
if [ -f "$file" ]; then
mv "$file" "${file}.skip"
echo " Excluded: $file"
fi
done
else
echo " No diagram files found to exclude"
fi
```
</issue_to_address>
### Comment 2
<location> `.github/workflows/build-and-publish.yml:131-140` </location>
<code_context>
+ # Exit with validation error code if validation failed
</code_context>
<issue_to_address>
**issue (bug_risk):** The comment claims to propagate the original validation exit code, but the script always exits with status 1.
The code always does `exit 1` rather than using `VALIDATION_EXIT_CODE`. If you intend to preserve the `vnext validate` exit code, you could do:
```sh
if [ "$VALIDATION_EXIT_CODE" -ne 0 ]; then
# ... messages ...
exit "$VALIDATION_EXIT_CODE"
fi
```
If you instead want to normalize all failures to `1`, please update the comment so it doesn’t imply the original validation code is propagated.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+96
to
+108
| # Temporarily rename diagram files to exclude them from validation | ||
| echo "📋 Excluding diagram files from validation..." | ||
| DIAGRAM_FILES=$(find core -name "*.diagram.json" -type f 2>/dev/null || true) | ||
| if [ -n "$DIAGRAM_FILES" ]; then | ||
| echo "$DIAGRAM_FILES" | while read file; do | ||
| if [ -f "$file" ]; then | ||
| mv "$file" "${file}.skip" | ||
| echo " Excluded: $file" | ||
| fi | ||
| done | ||
| else | ||
| echo " No diagram files found to exclude" | ||
| fi |
There was a problem hiding this comment.
suggestion (bug_risk): The echo ... | while read pattern is brittle for filenames with spaces and can be simplified.
echo "$DIAGRAM_FILES" | while read file will split on spaces and special characters, so any diagram path containing them may be processed incorrectly. You can avoid this by iterating directly over find’s output and preserving paths:
find core -name "*.diagram.json" -type f 2>/dev/null | while IFS= read -r file; do
mv "$file" "${file}.skip"
echo " Excluded: $file"
done
Suggested change
| # Temporarily rename diagram files to exclude them from validation | |
| echo "📋 Excluding diagram files from validation..." | |
| DIAGRAM_FILES=$(find core -name "*.diagram.json" -type f 2>/dev/null || true) | |
| if [ -n "$DIAGRAM_FILES" ]; then | |
| echo "$DIAGRAM_FILES" | while read file; do | |
| if [ -f "$file" ]; then | |
| mv "$file" "${file}.skip" | |
| echo " Excluded: $file" | |
| fi | |
| done | |
| else | |
| echo " No diagram files found to exclude" | |
| fi | |
| # Temporarily rename diagram files to exclude them from validation | |
| echo "📋 Excluding diagram files from validation..." | |
| DIAGRAM_FILES=$(find core -name "*.diagram.json" -type f 2>/dev/null || true) | |
| if [ -n "$DIAGRAM_FILES" ]; then | |
| find core -name "*.diagram.json" -type f 2>/dev/null | while IFS= read -r file; do | |
| if [ -f "$file" ]; then | |
| mv "$file" "${file}.skip" | |
| echo " Excluded: $file" | |
| fi | |
| done | |
| else | |
| echo " No diagram files found to exclude" | |
| fi |
Comment on lines
+131
to
+140
| # Exit with validation error code if validation failed | ||
| if [ $VALIDATION_EXIT_CODE -ne 0 ]; then | ||
| echo "" | ||
| echo "❌ vNext validation failed" | ||
| echo "Please check the validation errors above and fix them before proceeding" | ||
| echo "==========================================" | ||
| echo "Validation errors found in the following areas:" | ||
| echo "1. Schema files - Check if schema files are properly formatted" | ||
| echo "2. Task files - Check for missing required properties (tags, valid type values)" | ||
| echo "" | ||
| echo "Common issues to check:" |
There was a problem hiding this comment.
issue (bug_risk): The comment claims to propagate the original validation exit code, but the script always exits with status 1.
The code always does exit 1 rather than using VALIDATION_EXIT_CODE. If you intend to preserve the vnext validate exit code, you could do:
if [ "$VALIDATION_EXIT_CODE" -ne 0 ]; then
# ... messages ...
exit "$VALIDATION_EXIT_CODE"
fiIf you instead want to normalize all failures to 1, please update the comment so it doesn’t imply the original validation code is propagated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Exclude .diagram.json files from vNext validation in the build-and-publish workflow while preserving validation failure reporting.
Build:
CI: