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

fix: .github/workflows retrofit to use Go modules #10382

Closed
wants to merge 1 commit into from
Closed

fix: .github/workflows retrofit to use Go modules #10382

wants to merge 1 commit into from

Conversation

odeke-em
Copy link
Collaborator

Fixes #10319

Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@likhita-809 likhita-809 left a comment

Choose a reason for hiding this comment

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

lgtm

@tac0turtle tac0turtle added the A:automerge Automatically merge PR once all prerequisites pass. label Oct 19, 2021
@github-actions github-actions bot added the C:Cosmovisor Issues and PR related to Cosmovisor label Oct 19, 2021
- name: Create a file with all the pkgs
run: go list ./... > pkgs.txt
run: find . -name "go.mod" | grep -v cosmovisor | while read F;do cd $(dirname $F);go list ./...;cd $OLDPWD;done >> pkgs.txt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

Thanks for digging into it.
However we need to think a bit more. This doesn't solve the problems discussed in the original PR. As an evidence - look at the failing tests.

@@ -115,7 +119,7 @@ jobs:
if: env.GIT_DIFF
- name: test & coverage report creation
run: |
cat pkgs.txt.part.${{ matrix.part }} | xargs go test -mod=readonly -timeout 30m -coverprofile=${{ matrix.part }}profile.out -covermode=atomic -tags='norace ledger test_ledger_mock'
cat pkgs.txt.part.${{ matrix.part }} && cat pkgs.txt.part.${{ matrix.part }} | while read pkg;do go test -timeout 30m -coverprofile=${{ matrix.part }}profile.out -covermode=atomic -tags='norace ledger test_ledger_mock' $pkg;done
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't work for the same reason noted in #10325 (comment).

https://github.com/cosmos/cosmos-sdk/pull/10325/files#r729729294 I was thinking to do more traversal approach. I will dig more in my PR.

@robert-zaremba
Copy link
Collaborator

Could you check my comment: #10325 (comment)
I propose to close this PR.

@robert-zaremba robert-zaremba mentioned this pull request Oct 22, 2021
19 tasks
@robert-zaremba
Copy link
Collaborator

Closing in favor of #10325 and #10446

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:Cosmovisor Issues and PR related to Cosmovisor T: CI
Projects
None yet
4 participants