replace dep with go mod

[alessio]

Closes: #3919 #3630

• Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
• Wrote tests
• Updated relevant documentation (docs/)
• Added a relevant changelog entry: sdkch add [section] [stanza] [message]
• rereviewed Files changed in the github PR explorer

---

For Admin Use:

• Added appropriate labels to PR (ex. wip, ready-for-review, docs)
• Reviewers Assigned
• Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

[alessio]

I need to disable unconvert because of
https://circleci.com/gh/cosmos/cosmos-sdk/90784?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

[codecov]

Codecov Report

Merging #3907 into develop will decrease coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #3907      +/-   ##
===========================================
- Coverage    60.26%   60.25%   -0.02%     
===========================================
  Files          196      196              
  Lines        14615    14615              
===========================================
- Hits          8808     8806       -2     
- Misses        5214     5216       +2     
  Partials       593      593

[jackzampolin]

This gets a big 👍, tACK, LGTM and also a comment that it reduces install time by ~3x

[liamsi]

I left a few comments which I think should be discussed & clarified. Also, it might be worthwhile to streamline this as much as possible across our repositories (tendermint/iavl/amino, ...).

[alexanderbez]

TestedACK

[jleni]

This comment is more about the overall process and migration to go mod than this specific PR so I copy over here:

A risk I see with the migration to go mod is that we can have mismatches. For instance:

• develop (gopkg.toml) vs this PR (go.mod)
• old PR (gopkg.toml) and future develop (with new go.mod)
• branches, etc.

git will not help much in these cases.

I would say, for the next few months, we should add a manual step to confirm that we don't make mistakes when mixing changes in old gopkg vs new go.mod files.

[jleni]

@alessio I deleted my comment so I could move it over to the main thread and somehow it logs that I deleted yours, hehe :) Please repeat if I deleted something by mistake

[jleni]

it seems that something changed in the meantime :|

[alessio]

Checks should be back green shortly @jleni

[liamsi]

Speaking of caching. Is there a particular reason to not cache the dependencies. This would probably speed up builds inside of circle ci:

      - restore_cache:
          keys:
            - go-mod-v1-{{ checksum "go.sum" }}

# ...

      - save_cache:
          key: go-mod-v1-{{ checksum "go.sum" }}
          paths:
            - "/go/pkg/mod"

https://circleci.com/blog/go-v1.11-modules-and-circleci/

[alessio]

@liamsi I agree with you, and although your example looks promising, my lack of skills in CircleCI pipelines design does not allow me to assess on the spot with enough confidence where I should drop those simple snippets. Things such as shortening build times fit into the category of (desired) optimisations, thus I'd focus on getting the main functionality of this PR over the line first. I'll ask our Experts (TM) @greg-szabo @mircea-c to take a look eventually 🙏️

[alessio]

All comments are addressed. I'm just waiting for at least one additional review to come through.

[liamsi]

I'm also not a circcleci expert but line 62 reads - *dependencies. And the dependencies anchor is defined by:

deps: &dependencies
  run:
    name: dependencies
    command: |
      export PATH="$GOBIN:$PATH"
      make go-mod-cache

Shouldn't make go-mod-cache already have been called there?

[alessio]

Removed

[liamsi]

It would be cool if this could serve as as a reference for other repo's (like tendermint etc). For this, I think we should:

• revisit and reconsider the cmd/gosum/main.go approach (ref. the discussion in Makefile sha1sum is not available on all OSs #3630 (comment))
• enable caching of modules in the circleci config (and use that cache) (ref: replace dep with go mod #3907 (comment))
• remove the working_directory from the circleci config (moves code out of go-path) and remove all the export GO111MODULE=on(see: https://github.com/cosmos/cosmos-sdk/pull/3907/files#r266167635 / https://circleci.com/blog/go-v1.11-modules-and-circleci/)
• quickly move to golangci-lint and re-enable unconvert (and maybe other linters; ref: Make each golangci-lint linter pass tendermint/tendermint#3262 as en example how this could look like)
• verify that no documentation mentions dep / Gopkg.toml etc. (e.g. see: https://github.com/cosmos/cosmos-sdk/blob/develop/docs/_attic/sdk/sdk-by-examples/simple-governance/running-the-application.md#installation)

We should probably merge this as is and address those in a follow-up PR though.

[mircea-c]

@liamsi I've created an issue in our repo to make the changes to CircleCi config. https://github.com/tendermint/devops/issues/207

[dokwon]

Does this build even if the project directory is outside of GOPATH? having some issues aligning packages on my end...

[dokwon]

@jackzampolin @alessio

[fedekunze]

@dokwon I had the same issue, you need to remove the repository from $GOPATH

[alexanderbez]

Strange. I can build the SDK both inside and outside my $GOPATH it just matters if you have your modules env set.

[jackzampolin]

yeah its GO11MODULES=true or something like that.

[dokwon]

Solved! It was a GOPATH issue. Thanks all!

…

On Fri, Apr 5, 2019, 2:37 AM Jack Zampolin ***@***.***> wrote: yeah its GO11MODULES=true or something like that. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3907 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABjx-I4etWoOzt_6CSrwWe1BKvrRiN1cks5vdjhrgaJpZM4b2lui> .