Skip to content

Conversation

@kon-angelo
Copy link
Contributor

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #46

Special notes for your reviewer:

Release note:

Update sdk version to v2

@kon-angelo kon-angelo requested a review from a team as a code owner November 13, 2024 12:46
@gardener-robot gardener-robot added needs/review Needs review size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 13, 2024
@ghost ghost added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 13, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 13, 2024
@kon-angelo
Copy link
Contributor Author

cc: @AndreasBurger @MartinWeindel

Copy link
Member

@MartinWeindel MartinWeindel left a comment

Choose a reason for hiding this comment

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

Thanks!
Looks good to me overall. Only one small change request to get rid of the old AWS sdk dependency.

go.mod Outdated
go 1.23.0

require (
github.com/aws/aws-sdk-go v1.55.5
Copy link
Member

Choose a reason for hiding this comment

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

Not needed anymore.

Suggested change
github.com/aws/aws-sdk-go v1.55.5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Must have missed something. Lemme check again

@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Nov 13, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 13, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 13, 2024
Copy link
Member

@MartinWeindel MartinWeindel left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes needs/review Needs review labels Nov 13, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 13, 2024
Copy link
Member

@AndreasBurger AndreasBurger left a comment

Choose a reason for hiding this comment

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

lgtm

for _, tag := range tags {
if aws.StringValue(tag.Key) == "Name" {
return aws.StringValue(tag.Value)
if ptr.Deref(tag.Key, "") == "Name" {
Copy link
Member

Choose a reason for hiding this comment

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

You could also use aws.ToString(), up to you.

@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 14, 2024
Copy link
Member

@MartinWeindel MartinWeindel left a comment

Choose a reason for hiding this comment

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

/lgtm

@MartinWeindel MartinWeindel added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 14, 2024
@ghost ghost added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 14, 2024
@MartinWeindel MartinWeindel merged commit 37dbbce into gardener:main Nov 14, 2024
1 check passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging size/M Denotes a PR that changes 30-99 lines, ignoring generated files. status/closed Issue is closed (either delivered or triaged)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate to aws-sdk-go v2

6 participants