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

Remove pkg/util #1

Closed
hasheddan opened this issue Aug 21, 2019 · 1 comment · Fixed by #113
Closed

Remove pkg/util #1

hasheddan opened this issue Aug 21, 2019 · 1 comment · Fixed by #113
Labels
enhancement New feature or request

Comments

@hasheddan
Copy link
Member

This issue has been created to track the work originally expressed in crossplane/crossplane#448

It's generally considered a mispattern in the Go world to create package names like util, or common. The preferred alternatives are typically one or more of:

  • Breaking out functions into targeted packages with self documenting names like strings, metadata, etc.
  • Tolerating the existence of duplicate instances of simple utility functions like util.ToLowerRemoveSpaces defined closer to where they're used.
  • Avoiding such extremely simple utility functions altogether.

Sources:

@hasheddan hasheddan added the enhancement New feature or request label Aug 21, 2019
@negz negz changed the title Consider breaking up pkg/util Remove pkg/util Sep 18, 2019
negz added a commit to negz/crossplane that referenced this issue Jan 31, 2020
crossplane/crossplane-runtime#1

Per the above issue the util package is deprecated. As best I can tell the
stacks package is the only user of these functions, so I suggest they live
where they're used.

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/provider-azure that referenced this issue Jan 31, 2020
crossplane/crossplane-runtime#1

Per the above issue the util package is deprecated. As best I can tell the AKS
cluster controller is the only user of this retry setting, so suggest we move it
to where it's used.

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/provider-aws that referenced this issue Jan 31, 2020
crossplane/crossplane-runtime#1

Per the above issue the utils package is deprecated. As best I can tell these
are the only uses of these functions, which duplicate existing client functions.

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/provider-azure that referenced this issue Jan 31, 2020
crossplane/crossplane-runtime#1

Per the above issue the util package is deprecated. As best I can tell this is
the only use of this very simple function, so I've inlined it.

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/crossplane-runtime that referenced this issue Jan 31, 2020
crossplane#1

Each of these functions were called by only a single package, so I've raised
PRs to move them to the packages that call them. All relevant PRs are cross
linked from the above issue

Signed-off-by: Nic Cope <negz@rk0n.org>
@negz
Copy link
Member

negz commented Jan 31, 2020

As of #112 we're down to two functions left in pkg/util:

  • GeneratePassword is used by a handful of cache and database managed resource controllers, and the latter is used by all bucket controllers. We might consider replacing it with https://godoc.org/github.com/sethvargo/go-password/password#MustGenerate
  • ConditionalStringFormat is used by the bucket controllers. I believe it's only used to set bucket names, and should thus be replaced by the crossplaneio/external-name annotation once we bump those controllers to the crossplane-runtime managed resource reconciler.

negz added a commit to negz/provider-azure that referenced this issue Jan 31, 2020
crossplane/crossplane-runtime#1

Per the above issue the util package is deprecated. As best I can tell this is
the only use of this very simple function, so I've rehomed it.

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/crossplane-runtime that referenced this issue Jan 31, 2020
crossplane#1

Each of these functions were called by only a single package, so I've raised
PRs to move them to the packages that call them. All relevant PRs are cross
linked from the above issue

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/provider-gcp that referenced this issue Jan 31, 2020
crossplane/crossplane-runtime#1

Per the above issue GeneratePassword is one of two functions that remain in our
deprecated util package. I like go-password because it opens up the potential
to switch to a mock implementation for testing.

Signed-off-by: Nic Cope <negz@rk0n.org>
@negz negz closed this as completed in #113 Feb 1, 2020
wolffbe pushed a commit to wolffbe/provider-aws that referenced this issue Feb 12, 2021
crossplane/crossplane-runtime#1

Per the above issue the utils package is deprecated. As best I can tell these
are the only uses of these functions, which duplicate existing client functions.

Signed-off-by: Nic Cope <negz@rk0n.org>
namku pushed a commit to namku/provider-aws that referenced this issue Mar 9, 2021
crossplane/crossplane-runtime#1

Per the above issue the utils package is deprecated. As best I can tell these
are the only uses of these functions, which duplicate existing client functions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants