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

Better dev setup #79

Merged
merged 12 commits into from
Feb 17, 2022
Merged

Better dev setup #79

merged 12 commits into from
Feb 17, 2022

Conversation

mmlb
Copy link
Member

@mmlb mmlb commented Feb 14, 2022

QoL additions

Nix for its wonderful packaging/better reproducibility.
Go based tools are tracked via go modules using tools.go setup.
A makefile because its just nice and easy and well known.
I'm using tinkerbell/lint-install because it integrates with Makefiles well and like the way it configures the tools it uses.
Replaces non-functional TravisCI with GitHub Actions.

And then just a bunch of formatting/lint fixes.

go.mod Outdated
Comment on lines 5 to 14
require (
github.com/google/go-cmp v0.5.6 // indirect
github.com/google/uuid v1.2.0
github.com/kr/pretty v0.3.0 // indirect
github.com/rogpeppe/go-internal v1.8.1 // indirect
github.com/tinkerbell/lint-install v0.0.0-20220113213936-9b6f0db005b0
golang.org/x/sys v0.0.0-20211213223007-03aa0b5f6827 // indirect
golang.org/x/tools v0.1.8 // indirect
mvdan.cc/gofumpt v0.1.1
)
Copy link
Contributor

@thaJeztah thaJeztah Feb 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the new dependencies seem to be introduced because of the tools.go file. Would it be possible to move the tools.go to a submodule, so that it's dependencies do not end up in the main module's dependency?

Something like;

mkdir tools
mv tools.go tools/
go mod tidy

cd tools/
go mod init tools
go mod tidy

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah Is that actually a problem though? Having just the one module is easier to work with and it doesn't really have much of an effect on projects importing go-zfs, these deps only show up in go.sum.

[08:47:05]-[~/t/go-zfs-mod-tests]─[manny@zennix]> 
cat main.go go.mod
───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: main.go
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ package main
   2   │ 
   3   │ import (
   4   │     "fmt"
   5   │ 
   6   │     zfs "github.com/mistifyio/go-zfs/v3"
   7   │ )
   8   │ 
   9   │ func main() {
  10   │     datasets, err := zfs.Datasets("")
  11   │     if err != nil {
  12   │         panic(err)
  13   │     }
  14   │ 
  15   │     for _, ds := range datasets {
  16   │         fmt.Println(ds)
  17   │     }
  18   │ }
───────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: go.mod
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ module go-zfs-mod-tests
   2   │ 
   3   │ go 1.16
   4   │ 
   5   │ require github.com/mistifyio/go-zfs/v3 v3.0.0-20220213223527-e5563ba1dd28
   6   │ 
   7   │ replace github.com/mistifyio/go-zfs/v3 => github.com/mmlb/go-zfs/v3 v3.0.0-20220214015725-e51635b865aa
───────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
                                                                                                                                                                                      
[08:47:07]-[~/t/go-zfs-mod-tests]─[manny@zennix]> 
go mod tidy
                                                                                                                                                                                      
[08:47:09]-[~/t/go-zfs-mod-tests]─[manny@zennix]> 
git grep lint-install go.*
go.sum:github.com/tinkerbell/lint-install v0.0.0-20220113213936-9b6f0db005b0 h1:wArDwmgA90A4klrkNhV7vTZfNiP1Z7O7NWhpOl4bDZ8=
go.sum:github.com/tinkerbell/lint-install v0.0.0-20220113213936-9b6f0db005b0/go.mod h1:0h2KsALaQLNkoVeV+G+HjBWWCnp0COFYhJdRd5WCQPM=
                                                                                                                                                                                      
[--:--:--]-[~/t/go-zfs-mod-tests]─[manny@zennix]> 

With go.sum being a db of sorts I'm not really inclined to split off into 2 modules. Do you have a specific concern?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't really have much of an effect on projects importing go-zfs

Unfortunately, it does; go modules looks at what's defined in a dependency's go.mod and uses that to resolve what dependencies should be considered (and what version), even if the dependency is not used by the code you consume from the module you depend on 😞. We ran into issues because of that in various projects (e.g. the Cobra CLI-framework having some dependency on a grpc version, now forcing all projects to update their version).

Copy link
Member Author

@mmlb mmlb Feb 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah that makes sense. Will do what you mentioned then.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback @thaJeztah I've updated the PR and split into 2 modules, PTAL.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating! (Sorry for the extra hassle 😓)

I'll try to have a look soon (but quite busy currently, so don't hesitate to ping again 😅); I must add that I don't have much experience with Nix, but will try to see where I strand on that 😂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to use nix for anything here. Its mostly just the happy path. No worries, I'll likely merge soonish to keep the momentum going.

mmlb added 10 commits February 15, 2022 10:02
Tools are in a separate go module to avoid the tool's dependencies influencing
go mod's version decision when go-zfs is pulled into a different project.

From: mistifyio#79 (comment)
> @thaJeztah wrote:
> > @mmlb wrote:
> > > it doesn't really have much of an effect on projects importing go-zfs
> Unfortunately, it does; go modules looks at what's defined in a dependency's
> go.mod and uses that to resolve what dependencies should be considered (and
> what version), even if the dependency is not used by the code you consume
> from the module you depend on disappointed. We ran into issues because of
> that in various projects (e.g. the Cobra CLI-framework having some dependency
> on a grpc version, now forcing all projects to update their version).
The nicest thing lint-install brings is the versioned linter installs and
opinionated golangci-linter config file that was lint-clean on the go stdlib.
Based off of tinkerbell/boots.
Its defunct? Isn't running anymore...?
For better git diffs and treating each sentence as its own thing.
@mmlb mmlb merged commit d014733 into mistifyio:master Feb 17, 2022
@mmlb mmlb deleted the better-dev-setup branch February 17, 2022 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants