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

Consider a minor restructure of directory/file for util things #1336

Closed
carlisia opened this issue Aug 16, 2022 · 1 comment · Fixed by #1337
Closed

Consider a minor restructure of directory/file for util things #1336

carlisia opened this issue Aug 16, 2022 · 1 comment · Fixed by #1337
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@carlisia
Copy link
Contributor

Currently, there is a directory named "validation" that houses a folder named "util" and a package and file both named "utils". Nothing in this "utils" package does validation. There are only a couple things and they do some translation from Go types to pointer of custom types, which is entirely the same a currently open PR, which adds a file named "helper.go" to the root of the API directory (for both API versions) and adds the same type of translations.

Current layout:

├── apis
│   ├── doc.go
│   ├── v1alpha2
│   │   ├── doc.go
│   │   ├── gateway_types.go
|   |   |...more files....
│   │   ├── validation
│   │   │   ├── doc.go
│   │   │   |...more files....
│   │   │   └── util
│   │   │       ├── utils.go
│   │   │       └── utils_test.go

New PR: Add utils to translate custom types by abhijit-dev82 · Pull Request #1310 · kubernetes-sigs/gateway-api. An exception with this PR is that it also adds translation of the other way around (from custom type pointers to Go types).

That made me wonder if these translations should be split into separate packages. I don't think so. If not, then where, under the existing "validation" folder? One could argue that these are utils exclusively there to support the code that does validation. However, as the PR above signals, and in my own usage, that conversion functionality is very useful for interacting with the entire gateway api code base.

The problem I am thinking to solve:

  • keep like things together
  • have things be easily discoverable if we can help it

I propose moving the "util" folder out from under "validation" so they are parallel to each other. And rename things so both the package and folder match. I personally prefer util/util (singular).

The content of the "helper.go" file in the above PR should go in this package. The file itself could have a more precise name. Why name it something vague (helper, util) when everything in it is doing something very specific? The package could remain with the name util/utils so it servers as a holding place for future type of utility/helper things, but the file or files could be organized in a way as to reflect their contents. Could be that all of the existing + content of the PR mentioned would go in a file named "translation.go" or "conversion.go", or they could be split into separate files if it would make it helpful to make some further distinction for that content.

Proposal:

├── apis
│   ├── doc.go
│   ├── v1alpha2
│   │   ├── doc.go
│   │   ├── gateway_types.go
|   |   |...more files....
│   │   ├── validation
│   │   │   ├── doc.go
│   │   │   |...more files....
│   │   └── util
│   │       ├── conversion.go
│   │       └── conversion_test.go
@carlisia carlisia added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 16, 2022
@robscott
Copy link
Member

Thanks @carlisia! I think this would be a great change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants