CI: custom json formatter - #1501
Conversation
| REPO_PATH = Path(__file__).parent.parent.resolve() | ||
| EXCLUDE = [ | ||
| re.compile(rf"{REPO_PATH}/docs/.*"), | ||
| re.compile(rf"{REPO_PATH}/package-lock\.json"), | ||
| re.compile(rf"{REPO_PATH}/\.vscode/.*"), | ||
| re.compile(rf"{REPO_PATH}/\.devcontainer/.*"), | ||
| re.compile(rf"{REPO_PATH}/tests/data/.*/input\.json"), | ||
| re.compile(rf"{REPO_PATH}/tests/data/.*/.*_output.*\.json"), | ||
| re.compile(rf"{REPO_PATH}/tests/data/.*/update_batch\.json"), | ||
| re.compile(rf"{REPO_PATH}/tests/unit/deprecated/data/."), | ||
| ] |
There was a problem hiding this comment.
we need this, or there will be some ugly error-prone repeated code between the code quality workflow and this repo
There was a problem hiding this comment.
I'd argue that if we add this custom formatter, let's also include the following three
re.compile(rf"{REPO_PATH}/tests/data/.*/input\.json"),
re.compile(rf"{REPO_PATH}/tests/data/.*/.*_output.*\.json"),
re.compile(rf"{REPO_PATH}/tests/data/.*/update_batch\.json"),
I don't see what's the problem. We would get a BIG PR but it's just formatting.
There was a problem hiding this comment.
currently, we do a lot of compact formatting in our input/output/update. that would no longer be possible and also significantly increase test data size with literally only whitespaces due to indentation. this was also discussed when introducing biome
|
@mgovers I like to idea to get rid of npm. One alternative came into my mind to a custom JSON formatter: Biome has standalone binaries (it’s written in Rust). Unfortunately, this PR to publish Biome as Python package is currently not progressing biomejs/biome#8818 In summary I guess a custom JSON formatter - like your drafted one - might be the best solution (until there is a Python package for Biome). |
Hi @kornerc, Thank you for the input. We really appreciate it, especially after the great in-depth investigation you have done so far. 🤞 fingers crossed that that PR for biome will be published soon... |
ea9de81 to
881e110
Compare
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com> actually format json Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com> update script to exclude some Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com> cleanup Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com> minor Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com> exclude uv.lock, en/decode with utf-8-sig Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com> fix ruff Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com> re-add markdownlint-cli Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com> Refresh lock and linter dependencies Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Apply suggestions from code review Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com> Co-authored-by: Martijn Govers <martygovers@hotmail.com> Signed-off-by: Martijn Govers <martygovers@hotmail.com> minor Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
680e9f0 to
2489dd7
Compare
…e/custom-json-formatter
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…k-and-linter Refresh lock and linter dependencies
…e/custom-json-formatter
…e/custom-json-formatter Signed-off-by: Martijn Govers <martijn.govers@alliander.com>
figueroa1395
left a comment
There was a problem hiding this comment.
This is generally fine by me, but let's discuss with the rest of the team before proceeding.
| REPO_PATH = Path(__file__).parent.parent.resolve() | ||
| EXCLUDE = [ | ||
| re.compile(rf"{REPO_PATH}/docs/.*"), | ||
| re.compile(rf"{REPO_PATH}/package-lock\.json"), | ||
| re.compile(rf"{REPO_PATH}/\.vscode/.*"), | ||
| re.compile(rf"{REPO_PATH}/\.devcontainer/.*"), | ||
| re.compile(rf"{REPO_PATH}/tests/data/.*/input\.json"), | ||
| re.compile(rf"{REPO_PATH}/tests/data/.*/.*_output.*\.json"), | ||
| re.compile(rf"{REPO_PATH}/tests/data/.*/update_batch\.json"), | ||
| re.compile(rf"{REPO_PATH}/tests/unit/deprecated/data/."), | ||
| ] |
There was a problem hiding this comment.
I'd argue that if we add this custom formatter, let's also include the following three
re.compile(rf"{REPO_PATH}/tests/data/.*/input\.json"),
re.compile(rf"{REPO_PATH}/tests/data/.*/.*_output.*\.json"),
re.compile(rf"{REPO_PATH}/tests/data/.*/update_batch\.json"),
I don't see what's the problem. We would get a BIG PR but it's just formatting.
| echo "Running Biome" | ||
| npx biome format --write --files-ignore-unknown=true --no-errors-on-unmatched | ||
| echo "Running Json formatter" | ||
| git ls-files -z -- '*.json' '*.jsonc' | xargs -0 -r uv run --frozen scripts/format_json.py |
There was a problem hiding this comment.
*.jsonc with comments or trailing commas cannot be loaded by json.loads().
Comment:
import json
print(json.loads(
"""
{
// comment
"a": 1
}
"""
))Trailing comma:
import json
print(json.loads(
"""
{
"a": 1,
}
"""
))Even if json.loads() would be able to loads .jsonc files with comments, the comments would get lost.
Therefore, it might be a good decision to not format .*jsonc files
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
|



In light of recent events, we are aiming to reduce our amount of dependencies - in particular the
npmones. Unfortunately, that means we need to revert some of the changes in #1256 in favor of small custom scripts.Relates to #1499