📦 refactor(library): migrate entry point of all modules to lib/index.js

[oscard0m]

📝 Summary

• Change entry point for all modules to lib/index.js
• Keep retrocompatibility but warn the user about a future deprecation

⛱ Motivation and Context

#113

[gr2m]

When I run the tests, I only see one message

src/verify/index.js is deprecated. Use lib/index.js instead.

I'd expect to log messages for all the module imports?

Also the message should clarify that it's about the require statement, maybe something like this?

const verify = require('@octokit/webhooks/verify') is deprecated. Use const { verify } = require('@octokit/webhooks') instead

The tests should test for dedeprecations. I'd make a separate test/integration/deprecations-test.ts file which only does the now deprecated import. You can use simple-mock to test that the correct message was logged. Once that is all working, you can adapt the existing tests in the other files to remove the deprecation messages by using the new import

[oscard0m]

When I run the tests, I only see one message

src/verify/index.js is deprecated. Use lib/index.js instead.

I'd expect to log messages for all the module imports?

Let me take a look

Also the message should clarify that it's about the require statement, maybe something like this?

const verify = require('@octokit/webhooks/verify') is deprecated. Use const { verify } = require('@octokit/webhooks') instead

I like it. Sounds good to me to clarify the named export way we are migrating to.

The tests should test for dedeprecations. I'd make a separate test/integration/deprecations-test.ts file which only does the now deprecated import. You can use simple-mock to test that the correct message was logged. Once that is all working, you can adapt the existing tests in the other files to remove the deprecation messages by using the new import

👌 To add deprecations-tests (.js for the moment I guess)
👌 To adapt existing smoke-test to use new imports

[oscard0m]

🚑 Applied major fix (the previous changes were completely wrong)
☑️ Applied improvement on deprecation message
☑️ Adapted test/integration/smoke-test.js to use new exposed entries
☑️ Created test/integration/deprecation-test to assure there is retrocompatibility

---

⚠️ It seems that tap is hidding under the carpet the depreation warnings:

npx tap test/integration/deprecation-test.js

🆚

node test/integration/deprecation-test.js

Also tested manually creating dummy js files and running them with node. i.e.:

const verify = require("./src/verify");
verify("1234", {}, "randomSignature")

---

I've been researching a way to make tap output the deprecation warning message but I did not find a way. Do you know a way maybe?

Thanks @gr2m

[gr2m]

You do a great job splitting up your work in meaningful commits, makes it a joy to review your changes 👍

I've been researching a way to make tap output the deprecation warning message but I did not find a way. Do you know a way maybe?

I would mock process.emitWarning the way we currently do here:

webhooks.js/test/unit/event-handler-on-test.js

Line 13 in d675137

simple.mock(console, "warn").callFn(function () {});

The function simple.mock returns a mock object, which you can use for asserts, e.g.

const mock =   simple.mock(process, "emitWarning").callFn(function () {});
// ...
t.equals(mock.callCount, 1)

I think that should work 😁

[oscard0m]

You do a great job splitting up your work in meaningful commits, makes it a joy to review your changes 👍

Thanks for the compliment @gr2m 😍

I've been researching a way to make tap output the deprecation warning message but I did not find a way. Do you know a way maybe?

I would mock process.emitWarning the way we currently do here:

webhooks.js/test/unit/event-handler-on-test.js

Line 13 in d675137

simple.mock(console, "warn").callFn(function () {});

The function simple.mock returns a mock object, which you can use for asserts, e.g.

const mock =   simple.mock(process, "emitWarning").callFn(function () {});
// ...
t.equals(mock.callCount, 1)

I think that should work 😁

Let me give it a try!

[oscard0m]

Let me give it a try!

It works! @gr2m 🤓 🍾

[gr2m]

great work 👍 I've tested it against latest probot and it still all works

[github-actions]

🎉 This PR is included in version 7.3.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[oscard0m]

great work 👍 I've tested it against latest probot and it still all works

Thanks once again :)