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 gomod support #116

Merged
merged 3 commits into from
Feb 10, 2023
Merged

add gomod support #116

merged 3 commits into from
Feb 10, 2023

Conversation

cameracker
Copy link
Collaborator

@cameracker cameracker commented Feb 7, 2023

This PR adds gomod support as an alternate to #97, and implements #85.

In #97 , we discuss a proposal to fork the repo to stay perpetually at major version 0 so as to not force users to provide maintenance on their import paths. It's been a year since that proposal was opened and there's been no major steps to move us to a different repo or incorporate modules. In the meantime, the go ecosystem has moved on to grow the maturity of go.mod and it is increasingly making less sense for us to not comply with the standard package management for go.

Irrespective of the history this module has with go.mod (see notorious #61 and #65), the status quo has changed and consuming packages are no longer in flux with glide, dep, and gomod in a way that we need to support all at once. In this PR we take the appropriate cautious (potentially unnecessarily so) step and incorporate gomodules as a major version increment in order to bypass strange problems with import versioning.

To provide a complete proposal for merging this PR, here are some pros and cons.

Pros:

  • This package is now in conformance to the official package management tool for the go ecosystem
  • This resolves a perceived disincentive to adopt this library - the incompatibility with gomodules/
  • This change is friendlier to users' development environments
  • Gomod will help us enforce minimum supported versions of go

Cons:

  • Users will need to include the major version in their import paths
  • We need to be extra careful about unnecessary breaking changes
  • We folded to 'the man' ;)

For the first con, consumers who are unhappy with this change should strongly consider wrapping their usage of gofrs/uuid in a UUID module so that they only have to update one import path when they upgrade this repository.

Additionally, for maintainers of this library, it is important for us to be aware of but not paralyzed by the impact of making breaking changes to the library. But we've already been cautious and disciplined about making breaking changes. We have not made one in 2 years, and even though we're experimenting with v6 ad v7 UUIDs, there's nothing indicating that we are going to need to break the API anytime soon. Still, users can mitigate the blast radius by wrapping this package if they really are concerned about the imports becoming noisy.

I'll leave this PR open for a week to collect feedback.

go.mod Outdated Show resolved Hide resolved
@dylan-bourque
Copy link
Member

dylan-bourque commented Feb 7, 2023

Even though I'm unhappy about the cons, the logic here is sound. We will have to convert to a module at some point, especially if not doing so is leading to pushback on adoption, and the jump to V5 is, unfortunately, a necessary evil.

go.mod Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (2b8f2fa) compared to base (8345c9a).
Patch has no changes to coverable lines.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #116   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          498       498           
=========================================
  Hits           498       498           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@dylan-bourque dylan-bourque left a comment

Choose a reason for hiding this comment

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

LGTM, but it would probably be a good idea to update the Recommended Package Version section in README.md to point people at github.com/gofrs/uuid/v5 (with an explanation about why it's V5)

@cameracker
Copy link
Collaborator Author

Thanks Dylan, I'll look at doing that. I'll ask for a review again if I make that change

@cameracker cameracker merged commit ba6b224 into master Feb 10, 2023
@cameracker cameracker deleted the maintenance/introduce-gomod branch May 12, 2024 19:00
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