Skip to content
Draft

Oxfmt #438

Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,5 @@ dist
test/msg-box
test/bookshop/gen
test/bookshop/.cdsrc.json

.claude/
23 changes: 23 additions & 0 deletions .oxfmtrc.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"$schema": "./node_modules/oxfmt/configuration_schema.json",
"arrowParens": "avoid",
"bracketSpacing": true,
"embeddedLanguageFormatting": "auto",
"htmlWhitespaceSensitivity": "css",
"insertPragma": false,
"jsxSingleQuote": false,
"printWidth": 120,
"proseWrap": "preserve",
"quoteProps": "as-needed",
"requirePragma": false,
"semi": false,
"singleQuote": true,
"tabWidth": 2,
"trailingComma": "none",
"useTabs": false,
"vueIndentScriptAndStyle": false,
"sortPackageJson": false,
"ignorePatterns": [
"*.md"
]
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

4 changes: 3 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ if (process.env.CI && process.env.HANA_DRIVER) {
config.testMatch = ['**/tracing-attributes.test.js', '**/passport.test.js']

if (process.env.HANA_PROM)
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'
})
Comment on lines +11 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Suggested change
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

}

module.exports = config
4 changes: 3 additions & 1 deletion lib/exporter/ConsoleMetricExporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ class ConsoleMetricExporter extends StandardConsoleMetricExporter {
// export other metrics
for (const tenant of Object.keys(other)) {
for (const [k, v] of Object.entries(other[tenant])) {
LOG.info(`${k}${tenant !== 'undefined' ? ` of tenant "${tenant}"` : ''}: ${inspect(v.length === 1 ? v[0] : v)}`)
LOG.info(
`${k}${tenant !== 'undefined' ? ` of tenant "${tenant}"` : ''}: ${inspect(v.length === 1 ? v[0] : v)}`
)
}
}
}
Expand Down
23 changes: 15 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,26 @@
"name": "@cap-js/telemetry",
"version": "1.6.0",
"description": "CDS plugin providing observability features, incl. automatic OpenTelemetry instrumentation.",
"homepage": "https://cap.cloud.sap/",
"license": "Apache-2.0",
"author": "SAP SE (https://www.sap.com)",
"repository": {
"type": "git",
"url": "git+https://github.com/cap-js/telemetry.git"
},
"author": "SAP SE (https://www.sap.com)",
"homepage": "https://cap.cloud.sap/",
"license": "Apache-2.0",
"main": "cds-plugin.js",
"files": [
"lib"
],
"main": "cds-plugin.js",
"scripts": {
"lint": "npx eslint .",
"fmt": "oxfmt",
"fmt:check": "oxfmt --check",
"test": "npx jest --silent"
},
"lint-staged": {
"*": "oxfmt --no-error-on-unmatched-pattern"
},
"dependencies": {
"@opentelemetry/api": "^1.9",
"@opentelemetry/core": "^1.30",
Expand All @@ -28,9 +33,6 @@
"@opentelemetry/sdk-trace-node": "^1.30",
"@opentelemetry/semantic-conventions": "^1.36"
},
"peerDependencies": {
"@sap/cds": "^8 || ^9"
},
"devDependencies": {
"@cap-js/cds-test": "^0",
"@cap-js/sqlite": "^1 || ^2",
Expand All @@ -48,7 +50,12 @@
"chai-as-promised": "^7.1.1",
"chai-subset": "^1.6.0",
"eslint": "^10",
"jest": "^29.7.0"
"jest": "^29.7.0",
"lint-staged": "^17.0.8",
"oxfmt": "^0.56.0"
},
"peerDependencies": {
"@sap/cds": "^8 || ^9"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

},
"cds": {
"requires": {
Expand Down