Skip to content

Conversation

@Prithpal-Sooriya
Copy link
Contributor

@Prithpal-Sooriya Prithpal-Sooriya commented Dec 15, 2025

Explanation

This PR addresses ESLint suppressions in packages/notification-services-controller as part of the ASSETS-2100 cleanup initiative.

  • Current state and why it needs to change: The specified files had existing ESLint suppressions for @typescript-eslint/explicit-function-return-type, id-length, and @typescript-eslint/naming-convention. These suppressions needed to be resolved to improve code quality and consistency.
  • Solution your changes offer and how it works:
    • In perp-notifications.ts, the createPerpOrderNotification function now has an explicit Promise<void> return type, and the catch block variable e was renamed to error to satisfy id-length.
    • In notification-api.ts and schema.ts, file-level eslint-disable @typescript-eslint/naming-convention comments were added.
  • Changes whose purpose might not be obvious: The eslint-disable comments for naming-convention are intentional. schema.ts is an auto-generated file from an OpenAPI specification, and notification-api.ts defines types directly derived from an external Notification API, both of which dictate their own naming conventions (e.g., snake_case) that cannot be changed without breaking compatibility or manual intervention that would be overwritten.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Open in Cursor Open in Web


Note

Removes ESLint suppressions by adding an explicit return type and standardizing error variable in perp-notifications.ts, adding file-level naming-convention disables to API-derived type files, and updating eslint-suppressions.json.

  • Notification Services Controller:
    • services/perp-notifications.ts: Add explicit Promise<void> return type to createPerpOrderNotification; rename catch variable to error.
    • types/notification-api/notification-api.ts: Add file-level eslint-disable @typescript-eslint/naming-convention with note about external schema.
    • types/notification-api/schema.ts: Add file-level eslint-disable @typescript-eslint/naming-convention and auto-generated file note.
  • ESLint suppressions:
    • Update eslint-suppressions.json to remove entries for the above files.

Written by Cursor Bugbot for commit 8ddabd4. This will update automatically on new commits. Configure here.

Co-authored-by: prithpal.sooriya <prithpal.sooriya@consensys.net>
@cursor
Copy link

cursor bot commented Dec 15, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

core Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cursor remove this file

Copy link

Choose a reason for hiding this comment

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

Taking a look!

Open in Cursor Open in Web

@Prithpal-Sooriya
Copy link
Contributor Author

@cursor update the packages/notification-services-controller/CHANGELOG.md changelog to reflect changes made.

@cursor
Copy link

cursor bot commented Dec 15, 2025

Done! I've completed both requests:

  1. Removed the core file - The accidental core dump file has been removed from the repository

  2. Updated the changelog - Added an entry to packages/notification-services-controller/CHANGELOG.md:

## [Unreleased]

### Changed

- Improve code quality by adding explicit return types and ESLint compliance ([#7479](https://github.com/MetaMask/core/pull/7479))

The changes have been committed and pushed to the PR. This should also fix the "Check changelog" CI failure since the changelog now includes the changes made in this PR.


Open in Cursor Open in Web

Learn more about Cursor Agents

- Remove accidentally committed core dump file
- Update changelog with ESLint cleanup entry for PR #7479
@Prithpal-Sooriya Prithpal-Sooriya marked this pull request as ready for review December 15, 2025 11:10
@Prithpal-Sooriya Prithpal-Sooriya requested review from a team as code owners December 15, 2025 11:10
@Prithpal-Sooriya Prithpal-Sooriya changed the title Notification controller eslint cleanup chore: Notification controller eslint cleanup Dec 15, 2025
juanmigdr
juanmigdr previously approved these changes Dec 15, 2025
salimtb
salimtb previously approved these changes Dec 15, 2025
@Prithpal-Sooriya
Copy link
Contributor Author

@cursor please remove the changelog changes, as these eslint changes does not impact the implementation

@cursor
Copy link

cursor bot commented Dec 15, 2025

Done! I've removed the changelog entry for this PR. The ESLint cleanup changes are internal code quality improvements that don't affect the implementation, so they don't need to be documented in the changelog.


Open in Cursor Open in Web

Learn more about Cursor Agents

ESLint cleanup changes don't impact implementation and don't need
to be documented in the changelog.
@cursor cursor bot dismissed stale reviews from salimtb and juanmigdr via a231d8d December 15, 2025 12:37
bergarces
bergarces previously approved these changes Dec 15, 2025
juanmigdr
juanmigdr previously approved these changes Dec 15, 2025
@Prithpal-Sooriya Prithpal-Sooriya added this pull request to the merge queue Dec 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Dec 15, 2025
@Prithpal-Sooriya Prithpal-Sooriya dismissed stale reviews from juanmigdr and bergarces via 77a6ab1 December 15, 2025 13:06
@Prithpal-Sooriya Prithpal-Sooriya force-pushed the cursor/notification-controller-eslint-cleanup-e4db branch from 77a6ab1 to f839f3c Compare December 15, 2025 13:52
@Prithpal-Sooriya Prithpal-Sooriya added this pull request to the merge queue Dec 15, 2025
Merged via the queue into main with commit 11cf87a Dec 15, 2025
285 checks passed
@Prithpal-Sooriya Prithpal-Sooriya deleted the cursor/notification-controller-eslint-cleanup-e4db branch December 15, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants