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

.circleci: remove go mod checks for go 1.12 #1944

Merged
merged 3 commits into from
Nov 15, 2019

Conversation

debnil
Copy link
Contributor

@debnil debnil commented Nov 15, 2019

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into two PRs
    otherwise).
  • This PR's title starts with name of package that is most changed in the PR, ex.
    services/friendbot, or all or doc if the changes are broad or impact many
    packages.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or fixes.
  • I've updated any docs (developer docs, .md
    files, etc... affected by this change). Take a look in the docs folder for a given service,
    like this one.

Release planning

  • I've updated the relevant CHANGELOG (here for Horizon) if
    needed with deprecations, added features, breaking changes, and DB schema changes.
  • I've decided if this PR requires a new major/minor version according to
    semver, or if it's mainly a patch change. The PR is targeted at the next
    release branch if it's not a patch change.

What

This PR removes the go mod-related checks for Go 1.12.

Why

Go 1.13-specific projects like Hubble creates divergence in the dependencies, and the spread of 1.13 in the larger community makes continued support of 1.12 less important in the longer term. Additionally, community best practices advise against running these checks on multiple Go versions.

Known limitations

N/A

@cla-bot cla-bot bot added the cla: yes label Nov 15, 2019
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

Looks good. I think we can remove the entire check though.


One note on why we're okay making this change from the PR description:

makes continued support of 1.12 less important

Go 1.12 support is still as important as supporting 1.13 as it's a supported version in our list of supported versions in the README, but I think the distinction is that these checks exist to keep our code files clean. The go.list file exists so that we're aware of what versions are in use in our applications when we build them ourselves and ship binaries. Since our binaries are built only with Go 1.13 having it with Go 1.13 values only is sufficient I think. If someone else builds with Go 1.12 from source we're not really able to guarantee consistency, I'm not sure if we need to mention that somewhere. It should be consistent, but technically the Go module resolver could pick a different version or something if something major changed between versions. I don't think it's worth maintaining two separate go.list files for each version. The go.list file is not relevant for the SDK packages where people are importing our code.

.circleci/config.yml Outdated Show resolved Hide resolved
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

👍

@debnil debnil merged commit 2d0dc2e into stellar:master Nov 15, 2019
@debnil debnil deleted the no-mod-twelve branch November 15, 2019 23:56
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.

2 participants