Skip to content

Revert "quick fix for CircleCI (#2279)"#3261

Merged
melekes merged 8 commits intodevelopfrom
anton/circle
Feb 7, 2019
Merged

Revert "quick fix for CircleCI (#2279)"#3261
melekes merged 8 commits intodevelopfrom
anton/circle

Conversation

@melekes
Copy link
Contributor

@melekes melekes commented Feb 6, 2019

This reverts commit 1cf6712.

Plus, switch from gometalinter to golangci-lint https://github.com/golangci/golangci-lint for speed and better support.

@melekes melekes requested a review from ebuchman as a code owner February 6, 2019 10:12
@liamsi
Copy link
Contributor

liamsi commented Feb 6, 2019

ref: #2279
background: saving cache for PR from forks was broken on circleci
Is this fixed now?

Copy link
Contributor

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

💎

@melekes
Copy link
Contributor Author

melekes commented Feb 6, 2019

Is this fixed now?

looks like it

@codecov-io
Copy link

codecov-io commented Feb 6, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@1809efa). Click here to learn what that means.
The diff coverage is 37.5%.

@@            Coverage Diff             @@
##             develop    #3261   +/-   ##
==========================================
  Coverage           ?   63.58%           
==========================================
  Files              ?      213           
  Lines              ?    17550           
  Branches           ?        0           
==========================================
  Hits               ?    11160           
  Misses             ?     5466           
  Partials           ?      924
Impacted Files Coverage Δ
blockchain/pool.go 80.88% <ø> (ø)
crypto/merkle/proof.go 40.35% <0%> (ø)
libs/common/colors.go 0% <0%> (ø)
p2p/switch.go 59.68% <0%> (ø)
consensus/state.go 78.52% <100%> (ø)
libs/events/events.go 98.05% <100%> (ø)
p2p/pex/pex_reactor.go 79.24% <100%> (ø)

- nakedret
- lll
- gochecknoglobals
- govet
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not pass govet? Wasn't this enabled in metalinter?
da33dd0#diff-b67911656ef5d18c4ae36cb6741b7965L252

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably because of the older version

Copy link
Contributor

Choose a reason for hiding this comment

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

Weird, I thought govet is just part of the golang distribution (the go vet command).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or maybe gometalinter was broken

- gochecknoglobals
- govet
- gocritic
- gosec
Copy link
Contributor

@liamsi liamsi Feb 6, 2019

Choose a reason for hiding this comment

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

same, wasn't this previously enabled?
da33dd0#diff-b67911656ef5d18c4ae36cb6741b7965L259

.golangci.yml Outdated
- gochecknoinits
- scopelint
- stylecheck
- deadcode
Copy link
Contributor

Choose a reason for hiding this comment

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

@liamsi
Copy link
Contributor

liamsi commented Feb 6, 2019

I think, we should merge this as is and do a follow-up PR which re-enables all previously running linters (vet, gosec, ...?) and do the changes that are necessary due to the newer versions of these linters (and decide / discuss if want to enable a few more).

@melekes
Copy link
Contributor Author

melekes commented Feb 6, 2019

Yep, created #3262

WARN [runner/nolint] Found unknown linters in //nolint directives: gotype
Copy link
Contributor

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

💎 💎

@melekes melekes merged commit fcebdf6 into develop Feb 7, 2019
@melekes melekes deleted the anton/circle branch February 7, 2019 06:28
iKapitonau pushed a commit to scrtlabs/tendermint that referenced this pull request Jul 16, 2024
Fixes tendermint#3261 

This bug only affects `v0.38.x`.

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [ ] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
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.

3 participants