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

Support for /v1/synthetics #228

Merged
merged 3 commits into from
Apr 15, 2019
Merged

Conversation

btoueg
Copy link
Contributor

@btoueg btoueg commented Mar 26, 2019

Follow up on #205

@btoueg btoueg changed the title [WIP] Support for /v0/synthetics Support for /v0/synthetics Apr 2, 2019
@btoueg
Copy link
Contributor Author

btoueg commented Apr 5, 2019

Please ping me before merging, as we are about to bump the version of the API, so I might still add a few commits today

@btoueg btoueg changed the title Support for /v0/synthetics Support for /v1/synthetics Apr 8, 2019
@rtyler
Copy link

rtyler commented Apr 8, 2019

Really looking forward to this coming in and unblocking Terraform support for Synthetics 👏

Copy link
Collaborator

@ojongerius ojongerius left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ojongerius
Copy link
Collaborator

@btoueg happy to merge when you are ready. Thanks for helping @dharada1 getting this over the finish line! 🙇

@irabinovitch
Copy link

@ojongerius Thanks! As you can see from a few recent PRs were generally looking to get more involved in the project. Let us know if theres anyway we (the Datadog team) can help with on going development.

@btoueg btoueg force-pushed the synthetics_checks branch 3 times, most recently from f1b3a8e to fde6f59 Compare April 9, 2019 08:44
synthetics.go Outdated Show resolved Hide resolved
@btoueg btoueg force-pushed the synthetics_checks branch from 97b8af2 to 2e0b696 Compare April 9, 2019 12:53
@ojongerius
Copy link
Collaborator

Awesome @irabinovitch! @btoueg let me know when you are happy for me to merge.

@btoueg
Copy link
Contributor Author

btoueg commented Apr 10, 2019

Hi @ojongerius, this PR is good to go, thanks!

@bkabrda
Copy link
Collaborator

bkabrda commented Apr 10, 2019

@btoueg would you mind squashing the commits a little bit? For example, commit [1] only adds a TODO line, which is AFAICS then removed in another commit. This is relevant in terms of how this PR evolved, but not relevant to the history of the project when this gets merged. I wouldn't actually mind if this whole PR was a single commit, as it brings in a new functionality as a whole and doesn't interact much with the rest of the codebase.

[1] cf85f6a

@btoueg
Copy link
Contributor Author

btoueg commented Apr 10, 2019

Squash merge option should make it trivial https://github.blog/2016-04-01-squash-your-commits/ but I'll do it in case you don't have access to this functionality

@btoueg btoueg force-pushed the synthetics_checks branch from ec066ec to 7dad6b0 Compare April 10, 2019 12:09
@bkabrda
Copy link
Collaborator

bkabrda commented Apr 10, 2019

@btoueg interesting, I wasn't aware that was possible :)

Either way, I don't have rights to merge (yet, should get them in couple of days) so ATM I'm just expressing my opinion. Thanks!

@btoueg
Copy link
Contributor Author

btoueg commented Apr 11, 2019

Gonna add one commit today

@btoueg btoueg force-pushed the synthetics_checks branch from 29e47c2 to d9e52ea Compare April 11, 2019 15:53
@btoueg
Copy link
Contributor Author

btoueg commented Apr 12, 2019

@bkabrda @ojongerius Is it possible to merge it ASAP?

@bkabrda
Copy link
Collaborator

bkabrda commented Apr 15, 2019

No further comments from anyone here, the PR looks solid. I'm going to merge it. Thanks! 👍

@bkabrda bkabrda merged commit efa96fe into zorkian:master Apr 15, 2019
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.

6 participants