Skip to content
This repository has been archived by the owner on Jan 19, 2020. It is now read-only.

[WIP] Client Interface + ES2015 classes #41

Merged
merged 2 commits into from
Nov 6, 2016

Conversation

vutran
Copy link
Member

@vutran vutran commented Oct 18, 2016

This PR adds a base interface class and migrates existing clients to utilize ES2015 classes. This will make it easier for people to write clients.

Custom configurations per bot is moved to an init() method which gets called upon instantiation but the public API should still be the same.

Open for ideas if anything can be made better or needs to change.

// like this...
const client = new FacebookMessengerClient()(bot);

// or like this
bot.use(new FacebookMessengerBot());

// and with custom config
bot.use(new FacebookMessengerBot(config));

@jcampbell05
Copy link
Collaborator

@vutran will we need to update bottr-cli to run node with ES6 support ?

@vutran
Copy link
Member Author

vutran commented Oct 18, 2016

@jcampbell05 Not necessarily. I believe this will work on node v4+. I guess what we need to determine is whether to support Node v0.12 or lower since those versions doesn't support any ES6 features.

My thoughts is to support LTS up to most current version which is v4-v6+ for all packages. What are your thoughts? (side note: we can setup travis and specify the target versions for tests).

@jcampbell05
Copy link
Collaborator

@vutran I don't mind, I don't have much knowledge to know which version most people use. So if v4 is the lowest LTS version then lets use that!

You okay to set that up ?

@vutran
Copy link
Member Author

vutran commented Oct 18, 2016

@jcampbell05 Yeah, let's keep this PR open. I'll try to do some cleanup this weekend. (trying to balance time with my own projects atm)

@vutran vutran changed the title Client Interface + ES2015 classes [WIP] Client Interface + ES2015 classes Oct 18, 2016
@vutran vutran force-pushed the add-client-interface branch 2 times, most recently from 09fae66 to abd22b2 Compare October 21, 2016 13:42
@coveralls
Copy link

coveralls commented Oct 21, 2016

Coverage Status

Coverage increased (+3.8%) to 90.625% when pulling ef69855 on vutran:add-client-interface into 4787b91 on Bottr-js:master.

@jcampbell05
Copy link
Collaborator

@vutran Could I merge this in so I can help you out ?

@vutran
Copy link
Member Author

vutran commented Nov 3, 2016

@jcampbell05 Sure, may need to rebase due to the merge conflicts though. I haven't had the time to clean this up yet since I've been busy lately transitioning to a new job.

@jcampbell05
Copy link
Collaborator

@vutran No worries :) there as been quite alot of changes thats why I'm thinking it will be easier. I'll handle the merges :)

@coveralls
Copy link

coveralls commented Nov 5, 2016

Coverage Status

Coverage increased (+6.2%) to 91.667% when pulling 7b5ab6e on vutran:add-client-interface into 56d34e9 on Bottr-js:master.

@vutran
Copy link
Member Author

vutran commented Nov 5, 2016

@jcampbell05 I had to rebase the branch since some of the underlying API of the client changed recently.

@coveralls
Copy link

coveralls commented Nov 5, 2016

Coverage Status

Coverage increased (+6.2%) to 91.667% when pulling 1b46f22 on vutran:add-client-interface into 56d34e9 on Bottr-js:master.

@jcampbell05
Copy link
Collaborator

@vutran awesome I'll merge this so we can continue working on it

@jcampbell05 jcampbell05 merged commit 6f74c1a into Bottr-js:master Nov 6, 2016
@vutran vutran deleted the add-client-interface branch November 6, 2016 17:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants