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

Further tweaks for #168 #169

Merged
merged 18 commits into from
Jun 4, 2019
Merged

Conversation

blag
Copy link
Contributor

@blag blag commented Jun 4, 2019

Final tweaks for #168. This should be merged into the issue-124/exit_on_failures branch before #168 is merged in. That PR is looking pretty good, but there were a lot of places we can/should refactor, and some places we absolutely needed to refactor so we can test the new changes.

This PR:

  • Adds large comment in slack_monkey_patch.js for future developers to help determine if the monkey patching is still necessary.
  • Removes Object.assign() polyfill from lib/slack-messages.js - this requires node.js version > 8.3.0
  • Breaks out the dummy Logger out from tests/dummy-robot.js into its own module
  • Add robot_name argument to dummy Robot mock, to better mirror actual Robot constructor
  • Rename api and client variables (both st2client.js instances) to api_client and auth_client in scripts/stackstorm.js
  • Coalesce multiple var declarations into one
  • Combine and rename hubotErrorCallback and exitProcessWithLog into LogErrorAndExit, which is now used to handle uncaught exceptions within Hubot, and is explicitly called when we cannot recover from re/connection attempts
  • Refactor loadCommands to not accept an options object parameter, since we should always simply die if we can't connect or load commands
  • Move promise = authenticate(); into an else block to prevent it from running while the robot is stopping
  • Use array unpacking (var [var1, var2, var3] = [1, 2, 3];) since we can now that we're using Node.js > 8
  • Refactor (eventsource) stream error handling to properly register a callback instead of overwriting the onerror handler
  • Remove ST2 stream error handler since we now have an uncaughtException handler for hubot
  • Rename eventsource's source parameter to stream, since that makes slightly more sense
  • Close down the eventsource stream instead of creating a new one that immediately gets shut down
  • Add a few unit tests to test if hubot is stopped if certain environment variables aren't valid

@blag blag requested a review from jinpingh June 4, 2019 07:52
@blag blag merged commit 189b2d0 into issue-124/exit_on_failures Jun 4, 2019
@blag blag deleted the issue-124/further-tweaks branch June 4, 2019 19:45
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.

2 participants