-
Notifications
You must be signed in to change notification settings - Fork 243
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
Create Incident, List Priorities, and headers in POST method support #122
Create Incident, List Priorities, and headers in POST method support #122
Conversation
a5a2cdb
to
51d7663
Compare
I think the method name should be CreateIncident not CreateIncidents since it creates a singular incident The naming of the type to pass into the CreateIncident should be CreateIncidentOptions to match other method signatures |
Since CreateIncidentResponse isn't returned anywhere (just used to unmarshal the response inside CreateIncident(s) ), should it be un-exported? (I struggled with same thing on my PR) |
@wdhnl agree to both your comments, will update. |
Will you have time to update this? |
@wdhnl dunzo. sorry about that got side tracked. |
@mattstratton can you evaluate this PR? |
Hey Bill I just realized I missed one of the recommendations. Will fix
tonight or first thing tomorrow
…On Wed, Jun 20, 2018 at 5:55 PM Bill Hathaway ***@***.***> wrote:
@mattstratton <https://github.com/mattstratton> can you evaluate this PR?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#122 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFYalp8UbDo7Tw5PtxnntBguPsdgbxMxks5t-sTKgaJpZM4UddPL>
.
|
@wdhnl renamed CreateIncidentValue to CreateIncidentOptions like suggested. |
awesome, thanks |
Submitting this PR which adds support for POSTing (creating) an incident along with GETing (listing) priorities in the new priorities feature. Willing to change around any struct naming or json construction logic at request.
Methods added:
client.CreateIncidents
client.ListPriorities
PR also plumbed header support into POST method on the client.