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

ec2 refactor, unit test correction #72

Merged
merged 8 commits into from
Jun 14, 2018
Merged

ec2 refactor, unit test correction #72

merged 8 commits into from
Jun 14, 2018

Conversation

prekoa
Copy link
Contributor

@prekoa prekoa commented Jun 14, 2018

No description provided.

Copy link
Member

@lpuskas lpuskas left a 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 {
Copy link
Member

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"),
Copy link
Member

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

Copy link
Contributor Author

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",
Copy link
Member

Choose a reason for hiding this comment

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

typo: sport price

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, thx

Copy link
Member

@lpuskas lpuskas left a comment

Choose a reason for hiding this comment

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

lgtm
thx, @prekoa

@lpuskas lpuskas merged commit 23acd4b into master Jun 14, 2018
@lpuskas
Copy link
Member

lpuskas commented Jun 14, 2018

Fixes #55

@lpuskas lpuskas deleted the 55 branch June 14, 2018 12:47
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.

None yet

2 participants