Skip to content
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

OBS-384: upgrade ESLint to v8 #6893

Merged
merged 4 commits into from
Feb 11, 2025
Merged

OBS-384: upgrade ESLint to v8 #6893

merged 4 commits into from
Feb 11, 2025

Conversation

toufali
Copy link
Contributor

@toufali toufali commented Feb 7, 2025

In order to support more modern ECMAScript required for the front-end refactor, we need to upgrade ESLint, which currently blocks merging of the new FE code.

Unfortunately, ESLint 9 dropped support for Node < v18.18 – we’re currently on Node v14. ESLint 8 still supports Node v14.

For additional context, see ticket: https://mozilla-hub.atlassian.net/browse/OBS-478

  • Upgrade ESLint to 8
  • Migrate ESLint config file to compatible format
  • Upgrade ESLint related Prettier packages

@toufali toufali requested a review from a team as a code owner February 7, 2025 20:14
@toufali toufali marked this pull request as draft February 7, 2025 20:14
@toufali toufali marked this pull request as ready for review February 10, 2025 20:20
@toufali toufali requested a review from willkg February 10, 2025 20:21
@willkg
Copy link
Contributor

willkg commented Feb 10, 2025

@toufali Can you adjust the commit messages to match our conventions?: https://socorro.readthedocs.io/en/latest/dev.html#git-conventions

In this case, the issues are obs issues, so it should be something like this:

obs-384: upgrade ESLint to v8

and:

obs-384: upgrade Prettier

"graceful-fs": "4.2.4",
"prettier": "2.1.2",
"prettier": "^3.5.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why define a minimum rather than pinning to a specific version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@willkg when it's not a critical dependency I often use a minimum in case of minor patches. But I'm happy to pin it if you prefer!

Copy link
Contributor

@willkg willkg Feb 11, 2025

Choose a reason for hiding this comment

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

We update this stuff rarely, so I'm wondering whether it's better to pin it and then re-enable dependabot to update things. That way it doesn't change without us approving it.

So in this case, I think we should pin it to specific versions rather than specify a minimum.

@toufali
Copy link
Contributor Author

toufali commented Feb 11, 2025

@toufali Can you adjust the commit messages to match our conventions?: https://socorro.readthedocs.io/en/latest/dev.html#git-conventions

In this case, the issues are obs issues, so it should be something like this:

obs-384: upgrade ESLint to v8

and:

obs-384: upgrade Prettier

Sorry @willkg – I assumed since they get squashed anyway that it didn't really matter, but I see now that the commits are linked to the jira ticket which helps with reviews. I will fix these and try to remember for next time!

@toufali toufali force-pushed the OBS-384/upgrade-eslint branch from e6602aa to 1d33b74 Compare February 11, 2025 17:44
@toufali toufali requested a review from willkg February 11, 2025 17:46
@willkg
Copy link
Contributor

willkg commented Feb 11, 2025

@toufali : We landed the PR that removes legacy ES from Socorro. Can you rebase this against main tip? Then I can review it.

@toufali toufali force-pushed the OBS-384/upgrade-eslint branch from 14b8d57 to d555f1e Compare February 11, 2025 20:03
@toufali
Copy link
Contributor Author

toufali commented Feb 11, 2025

@willkg – rebase done 👍

Copy link
Contributor

@willkg willkg left a comment

Choose a reason for hiding this comment

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

Looks good! The only change I'd like to make is for us to pin the dependencies in package.json to specific versions. Let's make that change and then we can land this.

@toufali toufali added this pull request to the merge queue Feb 11, 2025
Merged via the queue into main with commit a946a07 Feb 11, 2025
1 check passed
@toufali toufali deleted the OBS-384/upgrade-eslint branch February 11, 2025 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants