Skip to content
This repository has been archived by the owner on Oct 24, 2019. It is now read-only.

Dispatch Refactor #113

Merged
merged 6 commits into from
Feb 6, 2018
Merged

Dispatch Refactor #113

merged 6 commits into from
Feb 6, 2018

Conversation

k-mahoney
Copy link
Contributor

Addressing issues in https://github.com/mapbox/dispatch/issues/37, refactor to normalize function and test structure, as well as error messages and general style. There is still some mixes use of callbacks and promises, can set future goal of converting entirety to one or the other for readability.

/cc @matiskay @zmully

Copy link
Contributor

@matiskay matiskay left a comment

Choose a reason for hiding this comment

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

Looks good to me. I only added some comments to improve the readability of some conditions.

incoming/function.js Outdated Show resolved Hide resolved
incoming/function.js Outdated Show resolved Hide resolved
lib/slack.js Outdated Show resolved Hide resolved
lib/slack.js Outdated Show resolved Hide resolved
@k-mahoney k-mahoney requested a review from zmully January 25, 2018 20:56
@k-mahoney k-mahoney mentioned this pull request Jan 25, 2018
11 tasks
MESSAGE-SPEC.md Outdated Show resolved Hide resolved
MESSAGE-SPEC.md Outdated Show resolved Hide resolved
incoming/function.js Outdated Show resolved Hide resolved
MESSAGE-SPEC.md Outdated Show resolved Hide resolved
incoming/function.js Outdated Show resolved Hide resolved
incoming/function.js Outdated Show resolved Hide resolved
lib/slack.js Outdated Show resolved Hide resolved
@k-mahoney
Copy link
Contributor Author

Thanks for reviewing @matiskay @zmully - starting test deploys this morning.

@k-mahoney
Copy link
Contributor Author

Working on fixing issue with error handling in github functionality, problem with unexpected return from getIssues if none are found. https://github.com/mapbox/dispatch/blob/dec5af4304323dcc7a57e2401c418d5ae552ade2/lib/github.js#L28-L40

@k-mahoney
Copy link
Contributor Author

@zmully I think I'm good to merge, pending any corrections you want to make to the log schema here. Currently we are not logging on every successful dispatch-incoming run, but we do log on a successful GitHub issue ticket closure or PagerDuty escalation in dispatch-triage.

@k-mahoney k-mahoney merged commit e5309a6 into master Feb 6, 2018
@k-mahoney k-mahoney deleted the refactor branch February 6, 2018 19:49
This was referenced Feb 6, 2018
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.

3 participants