-
Notifications
You must be signed in to change notification settings - Fork 4
Dxp 117 dependency dashboard #99
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…xp-117-dependency-dashboard
WalkthroughThe changes update the GitHub Actions workflow to use newer versions of key actions and Node.js, upgrade several development dependencies in package.json to their latest major versions, remove the old ESLint configuration and ignore file, and introduce a new ESLint configuration file with tailored rules for JavaScript, TypeScript, and test files. Additionally, a standalone transaction-related enum is removed, consolidating transaction result statuses. Changes
Possibly related issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (2)
.github/workflows/publish.yml (1)
25-28: Pin Node with a minor wildcard and enable npm cache for faster jobs.
- Use
22.xinstead of plain"22"so you automatically get the latest security-patch release:- node-version: "22" + node-version: "22.x"
- While you are touching this, consider turning on the built-in npm cache:
- name: Setup Node.js uses: actions/setup-node@v4 with: node-version: '22.x' cache: 'npm'This typically cuts ~30-50 % off the install step.
package.json (1)
34-35: Stale Jest typings detected.
@types/jestbumped to 30 but the project does not listjestitself and uses Vitest for testing.
If nothing in the source actually relies on Jest globals, drop the dependency to speed up install and avoid type pollution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
.github/workflows/publish.yml(2 hunks)package.json(1 hunks)
🔇 Additional comments (1)
.github/workflows/publish.yml (1)
14-18: Confirm v2 inputs/outputs oftibdex/github-app-token.
v2is a breaking-change release; some input names and the output key have changed in previous major bumps.
Please double-check that:
app_id/private_keyare still the expected input names.- the output is still exposed as
token, because the next step uses
${{ steps.ci_bot_token.outputs.token }}.A silent change here will break the whole pipeline at runtime.
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: 1
🧹 Nitpick comments (3)
eslint.config.js (3)
68-84: Consider re-enabling some TypeScript rules for better code quality.Many important TypeScript rules are disabled, which reduces type safety and code quality:
@typescript-eslint/no-explicit-anyallowsanytypes@typescript-eslint/no-non-null-assertionallows non-null assertions (!)@typescript-eslint/ban-ts-commentallows@ts-ignorecommentsWhile this might be necessary for legacy code compatibility, consider gradually re-enabling these rules to improve type safety.
93-141: Reduce code duplication between TypeScript and test configurations.The test configuration (lines 93-141) duplicates most of the TypeScript configuration (lines 44-91). Consider extracting the common configuration to reduce duplication.
+const commonTypeScriptConfig = { + languageOptions: { + parser: typescriptParser, + parserOptions: { + project: ['./tsconfig.json'], + sourceType: 'module', + }, + globals: { + ...globals.node, + ...globals.browser, + ...globals.es2021, + ...globals.jest, + }, + }, + plugins: { + '@typescript-eslint': typescript, + import: importPlugin, + }, + rules: { + ...js.configs.recommended.rules, + ...typescript.configs.recommended.rules, + ...prettierConfig.rules, + // ... other rules + }, + settings: { + 'import/resolver': { + typescript: {}, + }, + }, +}; { files: ['**/*.ts'], - languageOptions: { /* ... */ }, - plugins: { /* ... */ }, - rules: { /* ... */ }, - settings: { /* ... */ }, + ...commonTypeScriptConfig, }, { files: ['**/*.test.ts', '**/*.test-d.ts', 'tests/**/*'], + ...commonTypeScriptConfig, + languageOptions: { + ...commonTypeScriptConfig.languageOptions, + globals: { + ...commonTypeScriptConfig.languageOptions.globals, + ...globals.vitest, + }, + }, },
10-19: Review the specificity of ignore patterns.Some ignore patterns seem overly specific (e.g.,
commitLint.config.tson line 17). Consider if these are necessary or if more general patterns would suffice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.eslintignore(0 hunks).eslintrc.cjs(0 hunks)eslint.config.js(1 hunks)package.json(1 hunks)src/types/transactions.ts(1 hunks)
💤 Files with no reviewable changes (2)
- .eslintignore
- .eslintrc.cjs
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
What
@release-it/conventional-changelogfrom ^8.0.2 to ^10.0.0@types/jestfrom ^29.5.13 to ^30.0.0eslintfrom ^8.57.0 to ^9.0.0eslint-config-prettierfrom ^9.1.0 to ^10.0.0eslint-import-resolver-typescriptfrom ^3.6.3 to ^4.0.0release-itfrom ^17.6.0 to ^19.0.0vitestfrom ^2.1.1 to ^3.0.0tibdex/github-app-tokenfrom v1 to v2eslint.config.jsusing the flat config format (replacing.eslintrc.cjs)globalspackage.eslintignoreinto the new config.eslintrc.cjsand.eslintignorefilesTransactionResultenum insrc/types/transactions.tsby consolidating duplicate definitionsWhy
.eslintrc.*files to the new flat config formatTesting done
Decisions made
TransactionResultenum definitions rather than creating separate enumsChecks
Reviewing tips
eslint.config.jsreplaces the old.eslintrc.cjsand.eslintignorefilesTransactionResultenum consolidation insrc/types/transactions.ts- duplicate definitions were mergedUser facing release notes
Updated development dependencies to their latest versions, improving security and maintaining compatibility with modern tooling. Migrated ESLint configuration to the new v9 format for better future compatibility. This is an internal maintenance update with no breaking changes to the public API.