Skip to content

Add support for new_host_delay in monitor definitions #82

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

Merged
merged 2 commits into from
Jan 31, 2017

Conversation

jszwedko
Copy link
Contributor

This field defaults to 300 if empty, but we also need to support setting
it to 0 so I chose to use a *int pointer.

I stumbled upon #56 which discussed the use of pointers to support optional fields which seemed to be leaning towards the approach I took here, but let me know what you think.

TestCreateAndDeleteMonitor is failing locally for me, showing a diff including:

                        -  Handle: (string) "",
                        -  Id: (int) 0,
                        -  Name: (string) ""
                        +  Email: (string) (len=30) "[email protected]",
                        +  Handle: (string) (len=30) "[email protected]",
                        +  Id: (int) 222578,
                        +  Name: (string) (len=13) "Jesse Szwedko"

I can dig in more, but this also fails on master (first including the changes from #81) so I'm hoping I'm just doing something wrong when running the tests.

@ojongerius
Copy link
Collaborator

Nice one! Your implementation matches my preferred solution to #56. The acceptance test were failing, which was raised fixed in #73 / #85.

@@ -0,0 +1,8 @@
package datadog

func Int(val interface{}) *int {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I originally started coding up the same implementation, but if your function is named Int, has as single purpose returning a pointer to an int (or it returns nil), why not change the signature to only accept an int like so?:

func Int(v int) *int { return &v }

(See https://github.com/zorkian/go-datadog-api/pull/83/files#diff-01127be9ed6b6352616a2f1c10bc3540R29 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I tend to agree. I'd borrowed the implementation from aws/aws-sdk-go (which I meant to call out in the PR description), but I think explicit is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Borrowed your implementation in c08c499

@jszwedko jszwedko force-pushed the host-delay branch 2 times, most recently from f0f8962 to c08c499 Compare January 31, 2017 03:09
Jesse Szwedko added 2 commits January 31, 2017 03:10
This field defaults to 300 if empty, but we also need to support setting
it to 0 so we use a *int pointer.
@ojongerius ojongerius merged commit b4db85d into zorkian:master Jan 31, 2017
@ojongerius
Copy link
Collaborator

Thanks @jszwedko!

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