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

v2: Add go.mod #487

Merged
merged 1 commit into from
Sep 20, 2018
Merged

v2: Add go.mod #487

merged 1 commit into from
Sep 20, 2018

Conversation

bep
Copy link
Contributor

@bep bep commented Sep 14, 2018

Note that I have no practical experience with Go Modules and major versions, but this should be correct according to https://github.com/golang/go/wiki/Modules

I will follow up with a PR for the master branch.

I would also appreciate if this could be followed by a version tag. I plan to finally get to the Blackfriday v2 upgrade in Hugo in the near future ...

@dmitshur
Copy link
Collaborator

dmitshur commented Sep 16, 2018

The canonical import path of blackfriday v2 is gopkg.in/russross/blackfriday.v2, per https://github.com/russross/blackfriday#installation. How does this intersect with that?

Edit: That said, it also says:

With package management you should import github.com/russross/blackfriday and specify that you're using version 2.0.0.

/cc @rtfb

@bep
Copy link
Contributor Author

bep commented Sep 16, 2018

I'm fairly new to Go Modules (I guess we all are), so I'm not sure what is considered best practice here.

I notice go-yaml is using module "gopkg.in/yaml.v2":

https://github.com/go-yaml/yaml/blob/v2/go.mod

But I'm not sure what added benefit that abstraction gives, esp. considering the network issues with GitHub vs gopkg.in last week.

@rtfb
Copy link
Collaborator

rtfb commented Sep 17, 2018

The way I read vgo proposals, I understood that semantic import versioning was meant to become the canonical way, superseding all the pre-'go with modules' approaches. Not sure how much has changed between the vgo prototype and what went into Go 1.11, but conceptually doesn't seem like much should have.

The only advantage I see to keep using the gopkg.in import is to treat it like a vanity import path and avoid having two separate recommendations for those who use "the traditional go get" and others who use dependency management. But it has a disadvantage of depending on the availability of gopkg.in. So I don't know, I'm a bit undecided, but leaning away from gopkg.in, since it seems like a few years down the road it should fade away...

@dmitshur
Copy link
Collaborator

dmitshur commented Sep 17, 2018

It seems that we need to ensure that the installation instructions work for GOPATH workspaces and in module-aware mode, but they don't necessarily have to be the same. For instance, for GOPATH, it can continue to be:

  • github.com/russross/blackfriday for v1
  • gopkg.in/russross/blackfriday.v2 for v2

For module-aware mode, we can do:

  • github.com/russross/blackfriday for v1
  • github.com/russross/blackfriday/v2 for v2

As long as we don't run into conflicting issues that prevent both those workflows from being supported in the same repository, that could be a reasonable way to move forward. Whatever we decide and go with, it needs to be documented so people don't have to guess what import path to use.

go.mod Show resolved Hide resolved
@bep
Copy link
Contributor Author

bep commented Sep 17, 2018

I think it would also help to consider what the end user wants:

  • If I'm using Go Modules I will probably stick to that (it's not like Glide, Go Dep etc. where you tend to change your mind when something better pops up).
  • Given that, I would much prefer to use github.com/russross/blackfriday/v2 if I could, as for me it would give me the most robust solution.

@rtfb
Copy link
Collaborator

rtfb commented Sep 20, 2018

Well, looks like we're in agreement to go ahead with this PR. The only remaining question is whether we want to update the README language somewhat, or are we okay with that as well? (I just noticed that the links to dep and Glide are broken on v2 README, so that needs updating anyway...)

@bep
Copy link
Contributor Author

bep commented Sep 20, 2018

I can update the READMEs once I get to test how this works. But to test how this works I need this one merged. I suggest that you merge this and create a new minor version for both master and v2.

@rtfb
Copy link
Collaborator

rtfb commented Sep 20, 2018

SGTM, in it goes. Do you need the minor versions just for the tests, or can they be done after?

@rtfb rtfb merged commit d3b5b03 into russross:v2 Sep 20, 2018
@bep
Copy link
Contributor Author

bep commented Sep 20, 2018

I suggest you just tag master + v2 now.

@rtfb
Copy link
Collaborator

rtfb commented Sep 20, 2018

OK, I'll try to do that ASAP, but not today. I want to look back and collect some sort of release notes for those tags. I'll ping you back.

@rtfb
Copy link
Collaborator

rtfb commented Sep 23, 2018

Hey @bep, I've tagged v2.0.1 and v1.5.2.

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.

3 participants