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

Feature/ADF-1417/Upgrade to Node 18 #170

Merged
merged 12 commits into from
May 11, 2023

Conversation

jsconan
Copy link
Contributor

@jsconan jsconan commented Apr 25, 2023

Related to: ADF-1417 and ADF-1419

Requires:

Summary

Update the devDependencies for supporting Node 18

Set Node 18 for the CI workflow.

Details

To properly support Node 18, the dev dependencies had to be updated and aligned.

Note: The lock file was generated with npm@6 so it would also work seamlessly with Node 14.

How to test

  • check the report on GitHub
  • make sure to have Node 18 installed (you can use nvm.sh for managing multiple versions)
  • install the package: npm i
  • run the unit tests: npm test, npm run test:cov, npm run coverage
  • test the bundling: npm run build
  • test the linting: npm run lint:src, npm run lint:test

@jsconan jsconan requested review from shaveko and rgomeztao April 25, 2023 11:59
@github-actions
Copy link

Coverage Report

Totals Coverage
Statements: 90.78% ( 2589 / 2852 )
Methods: 95.19% ( 594 / 624 )

Copy link

@rgomeztao rgomeztao left a comment

Choose a reason for hiding this comment

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

looks nice, and no reported issued here :), lets go!

@@ -45,7 +45,7 @@ const errorHandler = {
/**
* Get a context by it's name and create it if it doesn't exists
* @param {String} name - the context name
* @returns {Object} the handling context
* @returns {Object|undefined} the handling context
*/

Choose a reason for hiding this comment

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

i understand this task is just fixing the doc, so no issues, but wouldnt it be nice that this handler return null instead of undefined? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, actually it would be better to refactor the code in order to keep a consistent behavior.

But I did not want to change it, and only avoid adding lint error due to the version change

Copy link
Contributor

@shaveko shaveko left a comment

Choose a reason for hiding this comment

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

Works good on my environment

@github-actions
Copy link

Version

Target Version 2.0.1
Last version 2.0.0

There are 0 BREAKING CHANGE, 0 feature, 1 fix

@jsconan jsconan merged commit b53c44c into develop May 11, 2023
@jsconan jsconan deleted the feature/ADF-1417/upgrade-to-node18 branch May 11, 2023 10:05
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