akshaykarle / go-mongodbatlas

A Go client library for the MongoDB Atlas API
GNU General Public License v3.0
16 stars 20 forks source link

Add alert configurations #4

Closed stormsilver closed 6 years ago

stormsilver commented 6 years ago

Purpose

Provide support for alert configurations (https://github.com/akshaykarle/terraform-provider-mongodbatlas/issues/7)

Notable changes

akshaykarle commented 6 years ago

Thanks for giving it a shot @stormsilver. Let me know once you hear from the Mongo Atlas people. Could you also look at the failing tests?

codecov-io commented 6 years ago

Codecov Report

Merging #4 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master     #4   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          10     11    +1     
  Lines         252    305   +53     
=====================================
+ Hits          252    305   +53
Impacted Files Coverage Δ
mongodbatlas/mongodb.go 100% <100%> (ø) :arrow_up:
mongodbatlas/alert_configurations.go 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 952b60b...78b1128. Read the comment docs.

stormsilver commented 6 years ago

@akshaykarle Okay, I've addressed those failing tests. Sorry about that. I've heard back from the MongoDB Atlas support folks and they have acknowledged the bug on their side, but I don't think it will affect the code here (this code ought to just start working when they fix their bug).

I have blemished your otherwise perfect code coverage record. Since I'm not really familiar with Go, I'm not sure how to test the SlingLogger part which is really just for debugging purposes anyway. If you are able to provide me a little bit of direction I will take a stab at testing it; or if you have other thoughts (remove it, change it, etc) I'm certainly open to those as well.

I think this is ready to merge barring any changes regarding code coverage.

stormsilver commented 6 years ago

@akshaykarle this should be ready to merge now. The Atlas folks have released the fix for the bug and I have tested it and confirmed it works. Let me know if there is anything else for me to do here; otherwise as soon as this is merged I'll update my terraform plugin branch.

akshaykarle commented 6 years ago

Thanks @stormsilver. Can we just get rid of the DEBUG_SLING? I can then just merge it in.

stormsilver commented 6 years ago

@akshaykarle Done

akshaykarle commented 6 years ago

Thanks a lot @stormsilver :)