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

Redo adapter inheritance #191

Merged
merged 5 commits into from
Aug 6, 2019
Merged

Redo adapter inheritance #191

merged 5 commits into from
Aug 6, 2019

Conversation

blag
Copy link
Contributor

@blag blag commented Aug 3, 2019

This PR is on top of #190.

This PR slightly refactors the adapter inheritance for Slack-like adapters.

It adds in an intermediary SlackLikeAdapter for the Slack-like providers. This should make it easier to use DefaultAdapter methods in subclasses, like with what is done in SparkAdapter.formatData and SlackLikeAdapter.normalizeCommand.

This makes things a bit easier:

-ThisAdapter.super_.prototype.customMethod.call(...)
+DefaultAdapter.customMethod(...)

I also removed the normalizeAddressee from the MS Teams adapter, since it was an override of the same function from DefaultAdapter (and the MS Teams adapter inherits from DefaultAdapter). The two functions were identical, however, so it makes no sense to override that function. And since the two functions were identical, I didn't need to change the tests at all for this fix.

Once #190 is merged, I will either rebase and merge this with GitHub, or I will manually rebase this branch on that one, force push, and merge it. Done.

@blag blag requested a review from arm4b August 3, 2019 00:02
@blag blag mentioned this pull request Aug 3, 2019
@blag blag force-pushed the redo-adapter-inheritance branch from 6dd538e to 3a14cdd Compare August 5, 2019 15:49
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Please include a Changelog record for this PR, as well as missing record for the fix made in previous PR #190

@arm4b arm4b added the refactor label Aug 5, 2019
@blag
Copy link
Contributor Author

blag commented Aug 5, 2019

I fixed a few things in the changelog, and added PR references to the most recent changes, including #190.

@blag blag merged commit 11aa5c2 into master Aug 6, 2019
@blag blag deleted the redo-adapter-inheritance branch August 6, 2019 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants