Conversation
SummaryThe following content is AI-generated and provides a summary of the pull request: Integrate
|
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR is exploratory ("playing around with oxfmt") but has a concrete formatting regression in jest.config.js where the newly reformatted object literal has its key at column 0, suggesting the formatter output was applied incorrectly or the tool has a bug at that indentation level. The missing newline at EOF in the formatter's own config file is also worth addressing.
PR Bot Information
Version: 1.26.5
- Event Trigger:
pull_request.opened - Correlation ID:
9c9108ba-9281-4f2d-86e1-bfbfcbeeda47 - File Content Strategy: Full file content
- LLM:
anthropic--claude-4.6-sonnet
| process.env.cds_requires_telemetry_tracing = JSON.stringify({ | ||
| _hana_prom: process.env.HANA_PROM === 'true' | ||
| }) |
There was a problem hiding this comment.
Formatting: The object key _hana_prom is not indented - it's at column 0 instead of being indented inside the object literal, which is inconsistent and hard to read.
| process.env.cds_requires_telemetry_tracing = JSON.stringify({ | |
| _hana_prom: process.env.HANA_PROM === 'true' | |
| }) | |
| process.env.cds_requires_telemetry_tracing = JSON.stringify({ | |
| _hana_prom: process.env.HANA_PROM === 'true', | |
| }) | |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| "oxfmt": "^0.56.0" | ||
| }, | ||
| "peerDependencies": { | ||
| "@sap/cds": "^8 || ^9" |
There was a problem hiding this comment.
Bug: peerDependencies was moved to after devDependencies, which is unconventional, but more critically it was temporarily removed and re-added — verify the final file still contains @sap/cds as a peer dependency (it does at lines 57-59), but this reordering should be intentional.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| "ignorePatterns": [ | ||
| "*.md" | ||
| ] | ||
| } No newline at end of file |
There was a problem hiding this comment.
Best Practice: File is missing a trailing newline (\ No newline at end of file) - most linters and editors expect a newline at end of file, and it's inconsistent with the formatter's own intent.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
|
Superseded by #473, which adopts prettier properly (config matching house style, format/format:check scripts, CI check, repo-wide reformat as a reviewable commit). This spike explored oxfmt but never landed. |
|
Correction: superseded by #476 (adopts oxfmt properly — config from this spike, format/format:check scripts, CI check, no git hook, repo-wide reformat as a reviewable commit). |
Supersedes the #438 oxfmt spike — adopts **oxfmt** (`0.63.0`, pinned) properly. ## Config (`.oxfmtrc.jsonc`) Based on the #438 spike, verified against the `lib/*.js` house style: `singleQuote`, `semi: false`, `printWidth: 120`, `tabWidth: 2`, `trailingComma: none`, `arrowParens: avoid`. `ignorePatterns` excludes `*.md`, `node_modules`, `package-lock.json`, `CHANGELOG.md`, and `jest.config.js` (see coordination). ## Scripts - `format` → `npx oxfmt` (write is oxfmt's default) - `format:check` → `npx oxfmt --check` No git hook / husky / lint-staged — CI `format:check` + scripts only (the intrusive hook from the #438 spike is intentionally not carried over). ## Commits (reviewable split) 1. `chore: add oxfmt formatter tooling` — config + scripts + devDep + lockfile + CI step 2. `chore: apply oxfmt formatting` — repo-wide reformat (11 files, line-wrapping only; verified non-semantic via `git diff -w`) ## eslint coexistence `@sap/cds/eslint.config.mjs` is `recommended` + `no-unused-vars`/`no-console` only (no stylistic rules) → no conflict. `npm run lint` stays green. ## CI One line added to the `lint` job in `ci.yml`: `npm run format:check`. ## Verified `npm run format:check` ✅ (56 files) · `npm run lint` (--max-warnings=0) ✅ · `npm run test` → 53 pass / 14 skip, exit 0 · lockfile resolved from public npm (0 internal-registry URLs). ## Coordination Parallel PR #474 (jest→vitest) deletes `jest.config.js` — this PR excludes it from formatting (0-line diff confirmed) so no collision. `package.json` change here is additive (scripts + devDep); lockfile will conflict with #474 — whichever merges second rebases.
1 participant
playing around with https://oxc.rs/docs/guide/usage/formatter.html