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.

Sign up for GitHub

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

Jump to bottom

Refactor pagerduty #52

Merged
merged 31 commits into from
Oct 1, 2015
Merged

Refactor pagerduty #52

merged 31 commits into from
Oct 1, 2015

Conversation

technicalpickles
Copy link
Contributor

I'm trying not to change behavior at all, but just get things in a better place. Some goals so far:

  • general code cleanup
  • move PagerDuty API interactions to their own class
  • more idiomatic error handling

I'm considering this a work in progress still.

@technicalpickles
Start a pagerduty lib, move get to there
078286a
@technicalpickles
Move pagerDutyPut to the lib
eb1a227
@technicalpickles
move pagerDutyPost to the lib
0599309
@technicalpickles
Move pagerDutyDelete to the lib
d7ff892
@technicalpickles
Rename pagerDutyGet to pagerduty.get
a65b29b
@technicalpickles
Rename pagerduty.put to pagerDutyPut
26199b5
@technicalpickles
Rename pagerDutyPost to pagerduty.post
378f1ce
@technicalpickles
Rename pagerDutyDelete to pagerduty.delete
04141d4
@technicalpickles
Use scoped-http-client directly, and re-use logic
1a0b4ff
@technicalpickles
move pagerDutyIncident(s) to lib, and rename to getIncident(s)
bd553ce
@technicalpickles
remove old method names
4fe388c
@technicalpickles
integration, not intergration
82737dc
@technicalpickles
Scripts should check for missing environment, rather than the client …
9d6d55e 
…every time
@technicalpickles
log noop, instead of sending it
145befe
@technicalpickles
Fix missingEnvironmentForApi
7b60f0c
@technicalpickles
Start using callbacks with errors
e0f1544
@technicalpickles
Make getIncidents use 2 param callback, with error for first
eee9812
@technicalpickles
Update getIncident to use error param in callback
4d682c1
@technicalpickles
update put callback to use an err
17a68aa
@technicalpickles
Update delete to use error callback
8719a16
@technicalpickles
Update post to use error callback. Add HTTP errors the other methods
5d97dbd
@technicalpickles
Remove support for single-arg callbacks.
441d91b
@technicalpickles
Add getSchedules
af456a5
@technicalpickles
Only keep env variables used in the specific files. Make subdomain ac…
ea2a084 
…cessible.
@technicalpickles
Remove msg parameter everywhere
4a7ce35
@technicalpickles
remove last reference to robot
1211963
@technicalpickles
Remove last references to robot. become a class
77c0045
@stephenyeargin
Copy link
Member

Might have already considered it, but it may be tempting to split out the low-level transport stuff into its own module. There appears to be one already, but it doesn't have a lot of the service query stuff built into this package.

In any case, 😻 because it makes this one step closer to being able to do something with #49

@technicalpickles
Copy link
Contributor Author

I started splitting out some of the functionality into different scripts. I started with just the webhook since it's totally unrelated to the API commands. I'd like to split the others out, but I think this diff is starting to get big enough so will follow up with that separately.

technicalpickles added a commit that referenced this pull request Oct 1, 2015
@technicalpickles
Merge pull request #52 from hubot-scripts/refactor-pagerduty
f1d3919 
Refactor pagerduty
@technicalpickles technicalpickles merged commit f1d3919 into master Oct 1, 2015
@technicalpickles technicalpickles deleted the refactor-pagerduty branch October 1, 2015 15:24
@stevefranks stevefranks mentioned this pull request Oct 30, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@technicalpickles @stephenyeargin