~adnano/go-gemini-devel

4 3

Seeking feedback for version v0.1.15

Details
Message ID
<C9F8NLU4P0RV.2LZR2635CQTFU@nitro>
DKIM signature
pass
Download raw message
I plan on releasing version v0.1.15 of go-gemini soon.
This version brings some major new features and changes.
The most notable change is the addition of an explicit context.Context
argument in various places, including methods on Client and Server.
The context provided to Handler.ServeGemini by the server will terminate
when the request is terminated. This should make dealing with contexts
much easier, at the cost of some verbosity and a long function signature.
I'm open to suggestions for improving this.

Other somewhat large changes include:
- FileSystem replaced with io/fs.FS. This means we now require Go 1.16.
- ServeMux now supports matching hostnames and schemes as well as paths.
- certificate.Store is now in charge of retrieving certificates.

There were also a lot of minor tweaks and improvements.

I hope to release version v0.1.15 soon after ironing out the remaining
issues and polishing up the API and documentation. Later on I plan on
releasing version v0.2.0, which should bring a better guarantee of
stability.

I have tagged version v0.1.15-alpha. Any feedback would be appreciated.
Details
Message ID
<C9F92JK69JJ7.9EHPJB5FIYNK@taiga>
In-Reply-To
<C9F8NLU4P0RV.2LZR2635CQTFU@nitro> (view parent)
DKIM signature
pass
Download raw message
I love it. I based gemini://srht.pages on this WIP stuff. I'm looking
forward to updating it with contexts as well once you release those.

On Sun Feb 21, 2021 at 8:21 AM EST, Adnan Maolood wrote:
> The context provided to Handler.ServeGemini by the server will terminate
> when the request is terminated. This should make dealing with contexts
> much easier, at the cost of some verbosity and a long function
> signature.

You could also consider mimicking the net/http approach by stashing the
context in the Request type.

> - FileSystem replaced with io/fs.FS. This means we now require Go 1.16.

Already noticed this and worked around it, but I would like to humbly
request that in the future you take care with rushing in to adopt new Go
features. We run the latest Alpine stable in production and I prefer not
to pull down newer versions of packages unless it's strictly necessary.
They release biannually so the wait isn't too terribly long.

Anyway, nice work! I like the changes very much.
Details
Message ID
<C9F95QSF7ERT.3FKWA5DJDO5XJ@nitro>
In-Reply-To
<C9F92JK69JJ7.9EHPJB5FIYNK@taiga> (view parent)
DKIM signature
pass
Download raw message
On Sun Feb 21, 2021 at 8:41 AM EST, Drew DeVault wrote:
> I love it. I based gemini://srht.pages on this WIP stuff. I'm looking
> forward to updating it with contexts as well once you release those.

Thanks! You can try the new API in version v0.1.15-alpha.

> You could also consider mimicking the net/http approach by stashing the
> context in the Request type.

I could, however this makes creating derivative contexts more difficult.
One must call Request.Context(), and then call Request.WithContext with
the derived context, which must copy the request. Also note that the http
Handler API was in use before the introduction of contexts, so they had
to implement that API for backwards compatibility. Since we don't have
to worry about that yet, I think we can come up with a better approach.

Also, the documentation for the context package says:

	Do not store Contexts inside a struct type; instead, pass a Context
	explicitly to each function that needs it. The Context should be the
	first parameter, typically named ctx.

Another possible aproach would be to combine the ResponseWriter and
Request arguments into one.

> > - FileSystem replaced with io/fs.FS. This means we now require Go 1.16.
>
> Already noticed this and worked around it, but I would like to humbly
> request that in the future you take care with rushing in to adopt new Go
> features. We run the latest Alpine stable in production and I prefer not
> to pull down newer versions of packages unless it's strictly necessary.
> They release biannually so the wait isn't too terribly long.

I'll keep that in mind. I don't expect to adopt any new Go features
anytime soon.
Details
Message ID
<C9F9FL0VJ49K.I7N3GT2NU82J@taiga>
In-Reply-To
<C9F95QSF7ERT.3FKWA5DJDO5XJ@nitro> (view parent)
DKIM signature
pass
Download raw message
On Sun Feb 21, 2021 at 8:45 AM EST, Adnan Maolood wrote:
> I could, however this makes creating derivative contexts more
> difficult.  One must call Request.Context(), and then call
> Request.WithContext with the derived context, which must copy the
> request. Also note that the http Handler API was in use before the
> introduction of contexts, so they had to implement that API for
> backwards compatibility. Since we don't have to worry about that yet,
> I think we can come up with a better approach.

That's a good point. Yours is probably the better design. I don't really
mind having a context parameter in function signatures.
Details
Message ID
<168f40100869c8be973256d404fc207b577a7ab3.camel@wetterberg.nu>
In-Reply-To
<C9F8NLU4P0RV.2LZR2635CQTFU@nitro> (view parent)
DKIM signature
missing
Download raw message
Great work! I haven't had time to kick the tires as much as Drew, but I
looked through the changes yesterday. Big thumbs up on adding contexts
to the handlers, I have been missing that. I also look forward to being
able to use the nice new embedding feature whenever resources need to be
bundled with the binary.

I understand Drews more measured approach to upgrades and new features,
but I think that in the case of Go it's mitigated by two things: the
language isn't volatile (unlike something like Rust) so upgrading in
itself isn't that much of a burden; and it's compiled so there are no
new runtimes with potential system-wide effects that need to be deployed
to production servers, the effect is limited to workstations & build
servers (that usally employs some container tech to create stable build
envs). There shouldn't be any kind of quasi-religious "move fast"
principle to adopting new Go releases, but in this case I think that
it's worth it to adopt the interfaces of the new fs package.

I'm luke-warm on the sub-packageification of the module, in small
projects like this it's usually not necessary, and just adds extra
imports for faux modularisation. Though that is probably primarily a
matter of personal taste and aesthetics, so ¯\_(ツ)_/¯

Most of all: thank you for your continued work on the project!
Reply to thread Export thread (mbox)