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

move MockAdapter into hubot repository #1348

Merged
merged 1 commit into from
Jun 7, 2017

Conversation

gr2m
Copy link
Contributor

@gr2m gr2m commented Jun 7, 2017

This is in preparation of #1347. Having the declaration of MockAdapter inside the Hubot repository makes sense anyway, it is fairly simple and we depend on it in our tests, so we should have full control. The main problem is that hubot-mock-adapter does require('hubot/src/adapter') which I don’t think is officially supported, it should be require('hubot').Adapter instead.

If we want to keep a hubot-mock-adapter we should bring it up to date (e.g. it refers src/scripts in the README.md which doesn’t exist) and move it to the hubotio GitHub organization.

In #1347 we want to convert the tests independent of the source files, and the changes in this PR became necessary for the tests to still pass after converting all /src/* files to JavaScript.

@mose
Copy link
Member

mose commented Jun 7, 2017

It looks like it's working fine. I think that package still makes sense to be maintained for plugin authors, and having it revived in hubotio/ sounds good. But I see no problem integrating it in main hubot code in any case.

@gr2m
Copy link
Contributor Author

gr2m commented Jun 7, 2017

Is the module used somewhere else? If we decide to move it to our hubotio org I’d be happy to use that again in the tests with a follow up PR, but would like to merge this first so we can move forward with #1347

Copy link
Contributor

@bkeepers bkeepers left a comment

Choose a reason for hiding this comment

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

I think this is fine for now, but agree with @mose that it would be good to have this as an external adapter so others can reuse it. But we can save that for another day.

@gr2m gr2m merged commit 846aec6 into master Jun 7, 2017
@gr2m gr2m deleted the prepare-tests-for-javascript-conversion branch June 7, 2017 16:02
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