Bottr-js / Bottr

🤖 The world's simplest framework for creating Bots
http://bottr.co
MIT License
938 stars 86 forks source link

[WIP] Client Interface + ES2015 classes #41

Closed vutran closed 7 years ago

vutran commented 7 years ago

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 commented 7 years ago

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

vutran commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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)

coveralls commented 7 years ago

Coverage Status

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

jcampbell05 commented 7 years ago

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

vutran commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

Coverage Status

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

vutran commented 7 years ago

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

coveralls commented 7 years ago

Coverage Status

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

jcampbell05 commented 7 years ago

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