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

refactor: Move methods in pkg/clients/ into invidual util packages #1917

Merged
merged 7 commits into from
Oct 18, 2023

Conversation

MisterMX
Copy link
Collaborator

Description of your changes

This moves the individual pkg/clients/*.go files into their own pkg/utils/ packages to provide better modularization. It also makes imports a bit cleaner as the pkg/clients was imported in the code with many different names (awsclients, awsclient, clients, aws etc.).

There are no functional changes to the code. Only the imports have changed. In a very small amount of places the pointer function has been replaced with the respective one from k8s.io/utils/ptr.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

n.a.

Signed-off-by: Maximilian Blatt (external expert on behalf of DB Netz) <maximilian.blatt-extern@deutschebahn.com>
Signed-off-by: Maximilian Blatt (external expert on behalf of DB Netz) <maximilian.blatt-extern@deutschebahn.com>
Signed-off-by: Maximilian Blatt (external expert on behalf of DB Netz) <maximilian.blatt-extern@deutschebahn.com>
Signed-off-by: Maximilian Blatt (external expert on behalf of DB Netz) <maximilian.blatt-extern@deutschebahn.com>
Signed-off-by: Maximilian Blatt (external expert on behalf of DB Netz) <maximilian.blatt-extern@deutschebahn.com>
@MisterMX MisterMX requested a review from chlunde October 17, 2023 08:41
@chlunde
Copy link
Collaborator

chlunde commented Oct 17, 2023

I think we should ensure that the replaced functions have identical behaviour, it will be very hard to test if any of these changes have negative side effects?

        controller_test.go:239: r: -want, +got:
              &v1beta1.Topic{
              	TypeMeta:   {},
              	ObjectMeta: {Annotations: {"crossplane.io/external-name": "arn:aws:sns:ap-south-1:862356124505:some-topic"}},
              	Spec: v1beta1.TopicSpec{
              		ResourceSpec: {},
              		ForProvider: v1beta1.TopicParameters{
              			Region:         "",
              			Name:           "some-topic",
              			DisplayName:    &"some-topic-01",
            - 			KMSMasterKeyID: &"",
            + 			KMSMasterKeyID: nil,
            - 			Policy:         &"",
            + 			Policy:         nil,
            - 			DeliveryPolicy: &"",
            + 			DeliveryPolicy: nil,
              			FifoTopic:      nil,
              			Tags:           nil,
              			```

Copy link
Collaborator

@chlunde chlunde left a comment

Choose a reason for hiding this comment

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

Enssure we have identical behaviour for empty string vs nil

Signed-off-by: Maximilian Blatt (external expert on behalf of DB Netz) <maximilian.blatt-extern@deutschebahn.com>
Signed-off-by: Maximilian Blatt (external expert on behalf of DB Netz) <maximilian.blatt-extern@deutschebahn.com>
@MisterMX
Copy link
Collaborator Author

Sorry, I think I messed something up while search-replacing the pointer methods. It should now work without any behavioral changes.

@MisterMX MisterMX requested a review from chlunde October 17, 2023 17:04
@MisterMX MisterMX merged commit dd7f4a0 into crossplane-contrib:master Oct 18, 2023
9 checks passed
@MisterMX MisterMX deleted the refactor/client-to-utils branch October 18, 2023 08:19
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