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

Add Go modules #309

Merged
merged 4 commits into from Oct 8, 2019
Merged

Add Go modules #309

merged 4 commits into from Oct 8, 2019

Conversation

nathany
Copy link
Contributor

@nathany nathany commented Oct 5, 2019

What does this pull request do?

Add Go modules

Where should the reviewer start?

go.mod

How should this be manually tested?

n/a

@dylan-bourque
Copy link

One note here, it's usually a bad idea to opt in to modules without bumping the major version.

@nathany
Copy link
Contributor Author

nathany commented Oct 7, 2019

@dylan-bourque Thanks for the heads up. Do you have any pointers as to what could break? I was thinking of this being a minor release update until you chimed in.

@dylan-bourque
Copy link

It's been a while since https://gitlab.com/gifts/uuid/issues/61 so I don't recall all the details. IIRC it was a bigger problem if your repo was already v2+, but in the abstract enabling support for modules is a non-trivial change in behavior. One noticeable thing is that importers who aren't using modules will get the last commit where go.mod doesn't exist (so no big fixes for non-modules consumers).

In contrast, though, bumping to v2 would also mean that every importer would have to change their import path (adding /v2 at the end) to get the update.

TBH, I don't know what the right path is I just didn't want to not say anything.

@nathany
Copy link
Contributor Author

nathany commented Oct 7, 2019

That's confusing. So rather than getting the latest commit from master, no Go-modules users would be stuck on the current 1.4.7 until they switch? (Assuming they are using Go 1.11+)?

@dylan-bourque
Copy link

Maybe it's the other way around and non-modules importers always get HEAD and switching to modules would "jump back" to the tagged commit.

Either way, the confusion was a big part of the advice to bump major versions when enabling modules.

@nathany
Copy link
Contributor Author

nathany commented Oct 8, 2019

I'm pinging #modules on Slack for some advice. Thanks for bringing this up.

@dylan-bourque
Copy link

dylan-bourque commented Oct 8, 2019

I was going suggest exactly that.

@nathany
Copy link
Contributor Author

nathany commented Oct 8, 2019

"if you are adopting modules for the first time for a pre-existing repository or set of packages that have already been tagged v2.0.0 or higher before adopting modules, then the recommended best practice is to increment the major version when first adopting modules."

It looks like gofrs/uuid#61 was v3.

So I think we're okay.

Copy link

@adamwg adamwg left a comment

Choose a reason for hiding this comment

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

lgtm.

@@ -0,0 +1,5 @@
module github.com/fsnotify/fsnotify
Copy link

@AlekSi AlekSi Jan 30, 2021

Choose a reason for hiding this comment

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

@nathany Was it an intentional change from https://gopkg.in/fsnotify.v1, or a mistake?

EDIT: ok, found https://github.com/go-fsnotify/fsnotify

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An intentional change.

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.

None yet

4 participants