cameracker commented 1 year ago

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.

dylan-bourque commented 1 year ago

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.

codecov-commenter commented 1 year ago

Codecov Report

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

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

:mega: 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

```diff @@ 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](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gofrs). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gofrs)

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

cameracker commented 1 year ago

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

gofrs / uuid

add gomod support #116

Codecov Report