Skip to content

Conversation

@dustymabe
Copy link
Member

Our whitespace checker [1] will complain if there are newlines at the end of files and since we are now copying the generated lockfiles into the src/config repo in bump-lockfile [2] the whitespace checker is complaining about it.

json.dump() itself doesn't add a newline because it isn't required to [3]. Let's just explicitly do it here.

[1] https://github.com/coreos/repo-templates/blob/5386c91f3f4b8f81009997efecf28b24cc8597a0/find-whitespace/script.sh#L27-L40
[2] coreos/fedora-coreos-pipeline@0029f12
[3] https://stackoverflow.com/questions/54716132/why-is-json-dump-not-ending-the-line-with-n

Our whitespace checker [1] will complain if there are newlines
at the end of files and since we are now copying the generated
lockfiles into the src/config repo in bump-lockfile [2] the
whitespace checker is complaining about it.

json.dump() itself doesn't add a newline because it isn't required
to [3]. Let's just explicitly do it here.

[1] https://github.com/coreos/repo-templates/blob/5386c91f3f4b8f81009997efecf28b24cc8597a0/find-whitespace/script.sh#L27-L40
[2] coreos/fedora-coreos-pipeline@0029f12
[3] https://stackoverflow.com/questions/54716132/why-is-json-dump-not-ending-the-line-with-n
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a trailing newline to JSON files generated by cmd-import. The changes are correct, but could be improved. For consistency, meta.json generated in finalize_build (line 234) should also get a trailing newline. Also, the logic for writing JSON files with a newline is duplicated; I've left a comment suggesting a refactoring to a helper function to improve maintainability.

@dustymabe dustymabe merged commit edc4b96 into coreos:main Dec 19, 2025
5 of 6 checks passed
@dustymabe dustymabe deleted the dusty-cmd-import-newlines branch December 19, 2025 20:43
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.

2 participants