search

zserge / lorca

Build cross-platform modern desktop apps in Go + HTML5
MIT License
8.04k stars 535 forks source link

Use Go 1.16 embed #140

Closed Srinivasa314 closed 3 years ago

Srinivasa314 commented 3 years ago

Fixes #138

Srinivasa314 commented 3 years ago

Maybe this can be merged to a dev branch because it wont work for people using go 1.15 and below

timcolson commented 3 years ago

Interesting update, @Srinivasa314. If I understand correctly, the embed directive in 1.16 replaces the generated code strategy used for 1.15 (and lower) to create a pseudo file system?

@zserge - I just watched a video code review of naabu GoLang Port scanner, which noted the pkgs are in a v2 directory to isolate the new API. I'm curious if that approach would work for Lorca? (i.e. "v2" for go 1.16+)

zserge commented 3 years ago

Thanks for cleaning up the old "embed" approach, clearly Go 1.16 embed is the way forward.

I am still not sure about the v2. In the past I had enough troubles with various Go packages that sometimes broke compatibility in minor releases, or on the contrary - bumped major versions for no good reason forcing users to change all import paths.

To me, the removal of Embed() does not sound like a breaking change. Those who migrated to Go 1.16 would completely replace Embed with stdlib's //embed approach. If we make it a v2 - they will have to update all import paths, even though no actual API have changed (not they ones they kept using after Go 1.16).

Those, who are still on Go 1.15 or who would not use Go's //embed for some reason - most likely they have invested into using some alternative embedding tool (like go-bindata).

I assume that the majority of Lorca users would not notice Embed's removal (it's not even the library API, it's the "tooling" API). But if we bump the major version here - all users will notice it and would have to modify their code. Those who would notice - would be happy to replace it with //embed to simplify their code and the build system.

My vote is for keeping the major version, increasing the minor version.

P.S. Just in case, Lorca is not even a v1 yet 😉

zserge commented 3 years ago

One small thing to mention, if switching Lorca to Go 1.16 - please update the CI pipeline to use Go 1.16 as well.

timcolson commented 3 years ago

Thx @Srinivasa314 for quick update to make build checks pass! Thx @zserge for the thoughtful comments on versioning. I'm new to the Go ecosystem. Learning every day. :smile:

FWIW - I started with 1.16 on macOS and Raspbian. But, I also have Fedora 33 -> go v1.15 by default. Looks like Fedora 34 due ~Apr 2021 will have go v1.16, but I also installed it manually w/o issue. My two cents -- my guess is Lorca users are probably on the cutting edge already. I'd prefer latest/greatest. Maybe worth adding the "discussion" feature of GitHub so folks could discuss rather than create issues?