-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
Append new lines to end of json files before writing #849
Append new lines to end of json files before writing #849
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes involve a modification to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
pkg/utils/json_utils.go (1)
54-58
: LGTM! Clean implementation for newline handling.The implementation correctly handles both empty files and existing content, ensuring JSON files end with a newline character. This aligns well with common Unix text file conventions and linter expectations.
Consider these minor readability improvements:
+const newlineByte = '\n' // Ensure that the JSON content ends with a newline -if len(indentedJSON) == 0 || indentedJSON[len(indentedJSON)-1] != '\n' { - indentedJSON = append(indentedJSON, '\n') +if len(indentedJSON) == 0 || indentedJSON[len(indentedJSON)-1] != newlineByte { + indentedJSON = append(indentedJSON, newlineByte) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/utils/json_utils.go
(1 hunks)
🔇 Additional comments (1)
pkg/utils/json_utils.go (1)
54-58
: Verify JSON parsing compatibility
The newline addition after valid JSON is spec-compliant, but let's verify this doesn't affect consumers.
✅ Verification successful
Adding a newline to JSON output is safe and spec-compliant
The codebase scan shows that all JSON parsing is done using Go's standard json.Unmarshal
, which is tolerant of trailing whitespace including newlines. The change to append a newline character is safe because:
- JSON specification allows optional whitespace (including newlines) after valid JSON
- All JSON parsing in the codebase uses
json.Unmarshal
which handles trailing whitespace correctly - No custom JSON parsers or strict validation is used that would be affected
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for JSON parsing code in the codebase to verify compatibility
rg -l 'json\.(Unmarshal|Decode)' | while read -r file; do
echo "=== $file ==="
rg -A 3 'json\.(Unmarshal|Decode)' "$file"
done
Length of output: 1438
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @joshAtRula
These changes were released in v1.126.0. |
what
It's common for linters and pre-commit hooks to expect new lines at the end of files.
In the case of the pretty-printed json (used for the creation of the backend.tf.json files) these objects are now properly indented, but still lacking the new line at the end of the file.
This PR addresses this by checking that the file is either empty OR that the last object by length isn't already a new line, and appends it.
why
Better support for linting and pre-comming standards.
references
Summary by CodeRabbit