search

openfaas / golang-http-template

Golang templates for OpenFaaS using HTTP extensions
https://www.openfaas.com/
MIT License
106 stars 57 forks source link

Replace ioutil #67

Closed mrwormhole closed 2 years ago

mrwormhole commented 2 years ago

My actions before raising this issue

Any possibility to replace ioutil with non deprecated method? I can just use io package which is the most direct & explicit way after 1.16v, I can also see some variable naming a bit annoying like bErr, cErr, dErr instead of being just err

I am also a bit confused about having request and response OpenFaaS model in a separate repository over here: github.com/openfaas/templates-sdk/go-http

Is there a reason why it is there but not in this repository? The files seem very minimal to have it somewhere else. For the maintainability sake, I would pull that repo's go-http here and delete this openfaas/templates-sdk dependecy for Go templates

Expected Behaviour

No behaviour change, just a maintainability and a convention

Current Behaviour

No behaviour change, just a maintainability and a convention

Possible Solution

No solution needed, tidy up task

Steps to Reproduce (for bugs)

N/A

Context

N/A

Your Environment

Next steps

You may join Slack for community support.

alexellis commented 2 years ago

@MrWormHole I don't understand what issues you are facing. This sounds like preference / purism, but I want to check I'm not misunderstanding.

Can you explain clearly what problem(s) you're facing using the golang-http template?

mrwormhole commented 2 years ago

yes, it is mostly a convention thing but I don't see a reason to use deprecated methods after 1.16, it gives an impression of unmaintained templates. Also every time, I create faasd project I have to import this as handler github.com/openfaas/templates-sdk/go-http (this repository started to bother me, I am not sure if this is really required to have it there and not in this repo) and I would prefer something like openfaas.Request openfaas.Response rather than just a concrete copy of struct, I checked handler.Request and handler.Response models, they seem like providing small abstraction which is fine but I think naming would play an important part as well as providing it through pointers. For example, I was confused about which one to use at the beginning and went with golang-http-template but in reality golang-http-template should be the one with req and w paramaters and handler.Response & handler.Request one should be branded as golang-openfaas-template because OpenFaaS is the model abstraction behind it. Also if there was an additional property in the request/response models, a person needs to update 2 separate repositories to integrate that change.

It is solely my preference of reading the code, I might be nitpicking here but all I am caring for is a bit of aesthetics and impression of maintenance on these templates, it is not necessarily a functional issue, it is structural tidy-up and version changes. There is no functional issue. Can be closed if it is not needed.

It is just a similar issue when a developer imports a library and wonders what it has been used under the hood, then checks out the underlying code, realizes it uses Go 1.11 and pkg/errors then sees an ongoing issue related to "use Go 1.13 with errors.Is errors.As" which has been frozen for a long duration. It works as it is intended but it is outdated and no one seems to worry about refactoring it. I am not saying these templates are that old but still gives me the impression of coming late. It also feels unnatural to use std req & w under the middleware templates(which I am currently using most of the time)

alexellis commented 2 years ago

Also every time, I create faasd project I have to import this as handler github.com/openfaas/templates-sdk/go-http (this repository started to bother me, I am not sure if this is really required to have it there and not in this repo) and I would prefer something like openfaas.Request openfaas.Response rather than just a concrete copy of struct,

These would still need to exist somewhere, and need to be imported.

I don't really follow what alternative you're suggesting.

yes, it is mostly a convention thing but I don't see a reason to use deprecated methods after 1.16, it gives an impression of unmaintained templates.

Incorrect, we have hundreds of users running on many versions of Go. Forcing them onto Go 1.17 features will break compatibility immediately and cause problems for businesses.

At time of speaking, either package is suitable for production and does not present a compilation error. The former is more compatible and breaks for fewer users.

I might be nitpicking here

That's my sense. We'll take a note of the feedback and close this for now.