-
Notifications
You must be signed in to change notification settings - Fork 19
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
ec2 refactor, unit test correction #72
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please have a look at comments and fix where appropriate
"github.com/aws/aws-sdk-go/service/pricing" | ||
"github.com/banzaicloud/telescopes/productinfo" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
type DummyPricingSource struct { | ||
type dummy struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use a better name for this and document what it is intended for
return &ec2.DescribeAvailabilityZonesOutput{ | ||
AvailabilityZones: []*ec2.AvailabilityZone{ | ||
{ | ||
State: aws.String("available"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please check if there are constants in the ec2 library for the strings here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx
name: "success - create a new config instance", | ||
check: func(config *aws.Config) { | ||
assert.NotNil(t, config, "the config should not be nil") | ||
name: "successful - get current sport prices", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: sport price
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed, thx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
thx, @prekoa
Fixes #55 |
No description provided.