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

Pin versions with go.mod, similar to server #1060

Merged
merged 2 commits into from
Feb 23, 2021
Merged

Pin versions with go.mod, similar to server #1060

merged 2 commits into from
Feb 23, 2021

Conversation

Groxx
Copy link
Contributor

@Groxx Groxx commented Feb 18, 2021

The versioning strategy unfortunately is leaking some dependencies, and codegen isn't reproducible any more.
This update mimics how github.com/uber/cadence does dependencies, and is more "normal" for go modules anyway.

Implications of these changes include:

  • Some build and runtime dependencies have changed versions in relatively minor ways. This mostly brings them more in line with the versions used to generate code.
    • I have not tried to upgrade any back to the previous versions, but it would be easy to try.
    • Generated code appears identical.
  • Users may end up downloading more dependencies (e.g. staticcheck), maybe not, depending on how Go decides to handle non-buildable transitive dependencies. They've always intended to prune them implicitly, but I do not know if that is happening yet.
    • They do not appear to be constrained by these dependencies, however, as they have no actual import, and do not appear in their go.mod files. Upgrading or downgrading tools in a test project worked and did not require replace directives.
  • For good or bad, transitive dependencies of tools and this library are now bound together.
    • This is pretty common and not usually an issue, but if it becomes one: we can switch conflicting tools to a separate tools.mod and deal with the internal-only quirks that causes.
    • I can do that ^ right now if we want to be paranoid, and it'd eliminate the chance of tool versions impacting users, but it opens the door to version skew. Neither option is ideal ¯\_(ツ)_/¯

And since this does away with the "X commits / versions behind current" notification: I've added make deps (and make deps-all), which works the same way as in cadence-workflow/cadence/pull/4000. Hopefully it's understandable enough to modify easily.

@CLAassistant
Copy link

CLAassistant commented Feb 18, 2021

CLA assistant check
All committers have signed the CLA.

Comment on lines -40 to +39
go.uber.org/atomic v1.5.1
go.uber.org/atomic v1.4.0
Copy link
Contributor Author

@Groxx Groxx Feb 18, 2021

Choose a reason for hiding this comment

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

This downgrade is a bit odd. Possibly it's not imported locally + transitive dependencies changed and this is the new minimum?
It's very likely safe / good to upgrade to latest. Latest does make a breaking change though, disallowing comparison between atomic wrappers (which is a good thing, as that's racy behavior).

Groxx added a commit to cadence-workflow/cadence that referenced this pull request Feb 18, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
github-actions bot pushed a commit to cadence-workflow/cadence that referenced this pull request Feb 18, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
Groxx added a commit to cadence-workflow/cadence that referenced this pull request Feb 18, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
github-actions bot pushed a commit to cadence-workflow/cadence that referenced this pull request Feb 18, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
Groxx added a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
github-actions bot pushed a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
Groxx added a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
github-actions bot pushed a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
Groxx added a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
github-actions bot pushed a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
Groxx added a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
github-actions bot pushed a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
Groxx added a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
github-actions bot pushed a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
@coveralls
Copy link

coveralls commented Feb 19, 2021

Pull Request Test Coverage Report for Build 3cb9e29a-b43c-4d9f-8388-fd80127d350b

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.008%) to 74.924%

Files with Coverage Reduction New Missed Lines %
internal/internal_task_pollers.go 2 75.25%
Totals Coverage Status
Change from base Build 53eedecc-d2b1-4af1-9cf6-9507912a6795: 0.008%
Covered Lines: 9666
Relevant Lines: 12901

💛 - Coveralls

@Groxx
Copy link
Contributor Author

Groxx commented Feb 19, 2021

Current deps output for comparison:

❯ make deps
220 days: github.com/uber/jaeger-client-go  	v2.22.1+incompatible -> v2.25.0+incompatible
371 days: golang.org/x/net  	v0.0.0-20200114155413-6afb5195e5aa -> v0.0.0-20210119194325-5f4716e94777
466 days: github.com/opentracing/opentracing-go  	v1.1.0 -> v1.2.0
501 days: go.uber.org/atomic  	v1.4.0 -> v1.7.0
503 days: honnef.co/go/tools  	v0.0.1-2019.2.3 -> v0.1.1
573 days: github.com/uber/tchannel-go  	v1.14.0 -> v1.21.0
733 days: github.com/kisielk/errcheck  	v1.2.0 -> v1.5.0
740 days: github.com/stretchr/testify  	v1.3.0 -> v1.7.0
871 days: go.uber.org/zap  	v1.8.0 -> v1.16.0
908 days: github.com/uber-go/tally  	v3.3.1+incompatible -> v3.3.17+incompatible
954 days: golang.org/x/lint  	v0.0.0-20180428170328-470b6b0bb300 -> v0.0.0-20201208152925-83fdc39ff7b5
958 days: go.uber.org/goleak  	v0.10.0 -> v1.1.10
1047 days: go.uber.org/yarpc  	v1.29.1 -> v1.52.0
1059 days: go.uber.org/thriftrw  	v1.11.0 -> v1.26.0
1167 days: golang.org/x/time  	v0.0.0-20170927054726-6dc17368e09b -> v0.0.0-20201208040808-7e3f01d25324
1352 days: github.com/pborman/uuid  	v0.0.0-20160209185913-a97ce2ca70fa -> v1.2.1
1435 days: github.com/sirupsen/logrus  	v0.11.5 -> v1.8.0
1506 days: github.com/apache/thrift  	v0.0.0-20161221203622-b2a4d4ae21c7 -> v0.14.0

This covers tools and libraries.

github-actions bot pushed a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
github-actions bot pushed a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
github-actions bot pushed a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?
Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.
Groxx added a commit to cadence-workflow/cadence that referenced this pull request Feb 19, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

An alternative option is to use something like https://github.com/psampaz/go-mod-outdated, but unfortunately that one will just tell you _what_ updates are available, not how old they are. It's pretty good otherwise though.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?  Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.

---

The output currently shows the maximum of these two values, as it feels like a pretty good "badness" metric:
- how much time has passed between the current version and the latest version
- how much time has passed since the latest version was released

Either alone produces some non-great edge cases:
- with only the first:
    - a long-stable library with a fix today looks 100s of days old
    - a release -> immediate bugfix release looks 0 days old (e.g. `rsc.io/sampler`)
- with only the second:
    - a consistently-releasing library always looks "recent-ish".  e.g. prior to them using tags, golang.org/x/tools would usually look <7 days old, as `master` is updated frequently... even though we were a few months behind.

Ideally, IMO, this would check for both:
- how old is our current version? (which this does)
- how long have we ignored _any_ update?

But AFAICT that'd require custom requests to a goproxy, as we cannot get "current+1" versions, only "current" and "latest".
@Groxx Groxx merged commit 42f6cac into master Feb 23, 2021
@Groxx Groxx deleted the versions branch February 23, 2021 03:14
yux0 pushed a commit to yux0/cadence that referenced this pull request May 4, 2021
While building github.com/cadence-workflow/cadence-go-client/pull/1060, I realized I kinda missed the out-of-date warnings.
So here's something kinda similar.

An alternative option is to use something like https://github.com/psampaz/go-mod-outdated, but unfortunately that one will just tell you _what_ updates are available, not how old they are. It's pretty good otherwise though.

It may be worth adding e.g. a "nothing > 100 days" check to `make lint` and/or CI?  Otherwise I tend to see dependencies go un-upgraded for huge lengths of time.

---

The output currently shows the maximum of these two values, as it feels like a pretty good "badness" metric:
- how much time has passed between the current version and the latest version
- how much time has passed since the latest version was released

Either alone produces some non-great edge cases:
- with only the first:
    - a long-stable library with a fix today looks 100s of days old
    - a release -> immediate bugfix release looks 0 days old (e.g. `rsc.io/sampler`)
- with only the second:
    - a consistently-releasing library always looks "recent-ish".  e.g. prior to them using tags, golang.org/x/tools would usually look <7 days old, as `master` is updated frequently... even though we were a few months behind.

Ideally, IMO, this would check for both:
- how old is our current version? (which this does)
- how long have we ignored _any_ update?

But AFAICT that'd require custom requests to a goproxy, as we cannot get "current+1" versions, only "current" and "latest".
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.

5 participants