Code Review Tweaks #1
Conversation
fix other minor issues
| /** | ||
| * @private | ||
| */ | ||
| static processMethod(type, data) { |
There was a problem hiding this comment.
New helper method I created to abstract commonality of all of the static methods below
| return {type:"Button", data:obj}; | ||
| } | ||
|
|
||
| static isPromise(obj) { |
There was a problem hiding this comment.
I pulled this outside of the class since it wasn't class specific. It's added again below
| let finalText = convo._write.reduce((acc, text) => acc.sentence(text), new Say()).toString().trim(); | ||
| let finalSpeech = convo._speak.reduce((acc, text) => acc.sentence(text), new Say()).toString().trim(); | ||
| let promises = []; | ||
| let textWasPopulated = !!(finalText || finalSpeech); |
There was a problem hiding this comment.
In javascript, an empty string is falsey, so we don't need the additional variables to hold textIsNotEmpty and speechIsNotEmpty as long as you trim the strings above. If you don't want to trim them for some reason, you can undo the change, but given how you are using them, that doesn't seem necessary
There was a problem hiding this comment.
That's the Java dev in me not used to "falsey" things =). Thanks for the tip there.
| /* eslint-enable new-cap */ | ||
| static prepComplete(obj, action = 'close', options) { | ||
| if (isPromise(obj)) { | ||
| return obj.then(result => this.prepComplete(result, action, options)); |
There was a problem hiding this comment.
I renamed obj to result in the arrow function to avoid any ambiguities on which scoped obj was being used
| return r; | ||
| } | ||
| setAccessToken(token) { | ||
| if (!conv.user) { |
There was a problem hiding this comment.
conv is not defined in this scope. I assumed you meant this.conv
When you do the diff, enable the "Hide whitespace changes" setting to make it significantly easier to review