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

Update dependencies, fix linter options and other details #47

Merged
merged 14 commits into from
Mar 17, 2023

Conversation

KristjanESPERANTO
Copy link
Contributor

No description provided.

@ianperrin
Copy link
Owner

Hi @KristjanESPERANTO

Thanks for the pull request.

Unfortunately, the changes are failing as the package name has been renamed. Can you revert the package name change and try again?

@KristjanESPERANTO
Copy link
Contributor Author

Hi @ianperrin 🙂
Capital letters are not allowed in the name in the package.json. I forgot to make the adjustment in the package-lock.json as well. I've done that now. The test should no longer get stuck at this point 😃

@ianperrin
Copy link
Owner

Hi @ianperrin 🙂
Capital letters are not allowed in the name in the package.json. I forgot to make the adjustment in the package-lock.json as well. I've done that now. The test should no longer get stuck at this point 😃

Understood.

Will anyone who has installed the module need to update their config file to use the lowercase name? If so this is a breaking change and will need mentioning in the updating section and example corrected in the Using the module sections of the README.md

Finally, sometime ago, I had created tests/node_helper.test.js with the intention of writing some tests for the module. For completeness, I believe the module name needs changing there too

@KristjanESPERANTO KristjanESPERANTO marked this pull request as draft March 16, 2023 07:38
@KristjanESPERANTO
Copy link
Contributor Author

Users shouldn't even notice the change. I have corrected this in other modules without issues.

But the test process still hangs: npm ERR! Cannot read property 'f1-api' of undefined.

I would take a look at that in the next few days. Until then I'll put the PR on draft 🙂

@KristjanESPERANTO KristjanESPERANTO marked this pull request as ready for review March 16, 2023 23:21
@KristjanESPERANTO
Copy link
Contributor Author

The tests should now work 🙂 The module works with node 14, but the tests with node 14 fail. Since node 14 will be EOL next month, I thought we can omit the node 14 test 😅

@KristjanESPERANTO
Copy link
Contributor Author

BTW: There is no breaking change.

@ianperrin ianperrin merged commit 2fa8670 into ianperrin:master Mar 17, 2023
@ianperrin
Copy link
Owner

Great thanks @KristjanESPERANTO

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