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

Upgrade TypeScript, Jest, Prettier, etc. to modern versions #176

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

MeltingMosaic
Copy link
Collaborator

@MeltingMosaic MeltingMosaic commented Aug 12, 2021

What problem does this PR solve?
I found that my code wouldn't compile when using modern TS syntax (e.g. ?. and ?? were not available). Upgrading TS required upgrading the rest of the dev environment stack.

What does this PR do?
The versions of the dev dependencies were very old, so I updated them to the latest versions. I ran the linter and fixed the new issues

What testing was done?
Ran UTs.

@ghost
Copy link

ghost commented Aug 12, 2021

CLA assistant check
All CLA requirements met.

@MeltingMosaic MeltingMosaic marked this pull request as draft August 12, 2021 04:25
@MeltingMosaic MeltingMosaic marked this pull request as ready for review August 12, 2021 04:25
@MeltingMosaic MeltingMosaic marked this pull request as draft August 12, 2021 04:28
@MeltingMosaic MeltingMosaic marked this pull request as ready for review August 12, 2021 05:04
Copy link
Collaborator

@skiptirengu skiptirengu left a comment

Choose a reason for hiding this comment

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

👍

@@ -14,7 +14,7 @@ jobs:

strategy:
matrix:
node-version: [8.x, 10.x, 12.x]
node-version: [10.x, 12.x, 14.x]
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we're dropping support for older node versions, this new requirement should also be updated on the "engines" section of the package.json file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO we should probably also bump the major package version.

@schubboy
Copy link

Curious what the timeframe might be for getting this merged and released?

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.

None yet

3 participants