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

build(deps): upgrade eslint-config-airbnb-typescript and @typescript-eslint/* packages to latest #963

Merged
merged 4 commits into from
May 8, 2024

Conversation

davilima6
Copy link
Contributor

@davilima6 davilima6 commented Apr 8, 2024

Addresses .

Purpose

This PR upgrades a couple of linter-related libraries to facilitate the upgrade of Typescript to latest in consuming projects

Currently, using TS >= 5.4.0 results in a linter warning similar to this (incl. in this very repo):

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.
You may find that it works just fine, or you may not.
SUPPORTED TYPESCRIPT VERSIONS: >=4.3.5 <5.4.0
YOUR TYPESCRIPT VERSION: 5.4.5
Please only submit bug reports when using the officially supported version.

Approach and changes

  • upgraded eslint-config-airbnb-typescript to latest ^18.0.0
  • upgraded @typescript-eslint/{estlint-plugin,parser} to latest ^7.5.0 (maybe this needs to be instead ^6.22.0 || ^7.5.0? 🤔), fixing the error above

Definition of done

  • Development completed
  • Reviewers assigned
  • Unit and integration tests

@sumup-clark
Copy link

sumup-clark bot commented Apr 8, 2024

Hey @davilima6,
we are super excited that you are contributing! 🎉There's one more thing you need to do. Please accept our Contributor License Agreement. It helps you and us to collaborate on clear terms and focus on what we love most: code.

Thanks!

Copy link

changeset-bot bot commented Apr 8, 2024

🦋 Changeset detected

Latest commit: 4485f4f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sumup/foundry Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

codecov bot commented Apr 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.40%. Comparing base (ee8ca93) to head (4485f4f).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #963   +/-   ##
=======================================
  Coverage   55.40%   55.40%           
=======================================
  Files          29       29           
  Lines        2366     2366           
  Branches      138      138           
=======================================
  Hits         1311     1311           
  Misses       1037     1037           
  Partials       18       18           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hilleer
Copy link
Contributor

hilleer commented Apr 10, 2024

This would be super useful bump! 🙌

@davilima6
Copy link
Contributor Author

davilima6 commented Apr 10, 2024

I'm currently failing to sign the CLA to unblock this PR (the page doesn't seem to work). Needs actual testing too, and a changelog entry. Hopefully I'll have time to move on with it this week or by the beginning of the next one. In the meantime, if anyone wants to push this forward just let me know.

@connor-baer
Copy link
Member

connor-baer commented May 6, 2024

@davilima6 You don't need to sign the CLA since you're employed by SumUp.

The CLA bot has been broken for a while 😞 I'll make a PR to remove the requirement and discourage external contributions.

Edit: #969 has been merged and I've disabled the CLArk GitHub App. Pushing another commit should get rid of the failing check.

@connor-baer
Copy link
Member

This will have to be released as a breaking change since typescript-eslint v7 raised the minimum Node version requirement to v18.18+ in line with ESLint v9 (typescript-eslint/typescript-eslint#8377). Foundry currently requires v18.12+.

@davilima6
Copy link
Contributor Author

@connor-baer , thanks for having a look. Your merge commit has cleared the check failure.

I don't think there's a rush to release this, so we could batch with other breaking changes. Are there any others in sight?

@connor-baer
Copy link
Member

connor-baer commented May 7, 2024

@davilima6 ESLint v9 was released a month ago and I was hoping to upgrade to it with the next major. However, a lot of the plugins we use aren't compatible yet, so I'm open to releasing one sooner.

@davilima6
Copy link
Contributor Author

@connor-baer, got it. Since this breaking change shouldn't be a too hard issue for users (afaik), I bumped the versions to latest and added a changeset, so feel also free to edit it directly, as well as release whenever you think it's best. If you need any other adjustments, just let me know. I'll try to test later in our project using Yalc.

@davilima6 davilima6 marked this pull request as ready for review May 7, 2024 08:17
@davilima6 davilima6 requested a review from a team as a code owner May 7, 2024 08:17
@davilima6 davilima6 requested review from pdrmdrs and removed request for a team May 7, 2024 08:17
Copy link
Member

@connor-baer connor-baer left a comment

Choose a reason for hiding this comment

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

Thank you!

@connor-baer connor-baer merged commit 46a67f9 into main May 8, 2024
7 checks passed
@connor-baer connor-baer deleted the build/upgrade-ts-eslint branch May 8, 2024 17:51
@connor-baer connor-baer mentioned this pull request May 8, 2024
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.

3 participants