ci: use gofumpt because gofmt and goimports conflicted#9738
Closed
faddat wants to merge 6 commits intotendermint:mainfrom
Closed
ci: use gofumpt because gofmt and goimports conflicted#9738faddat wants to merge 6 commits intotendermint:mainfrom
faddat wants to merge 6 commits intotendermint:mainfrom
Conversation
Contributor
Author
|
I think that this is affected by the "flaky tests" |
Contributor
|
@faddat could you please share more context on the problem(s) you're trying to solve in more detail for us? |
Contributor
Author
|
@thanethomson sure! So, basically this PR changes just code formatting and the Makefile. this PR:
|
Contributor
|
When I run While I understand that you may find this to be an improvement @faddat, our team still does not, as per #8478 (comment). |
mergify bot
pushed a commit
that referenced
this pull request
Nov 24, 2022
## NOTE: this pr exclusively runs commands from the makefile found here This PR ONLY runs `make format` ... then `make mockery` Its purpose is to ensure that the review scope of other PR's, which changed .go files and thus triggered the linter that only runs conditionally, have smaller review scopes, and should be merged before: #9738 #9739 #9742 --- #### PR checklist - [x] Tests written/updated, or no tests needed - [x] `CHANGELOG_PENDING.md` updated, or no changelog entry needed - [x] Updated relevant documentation (`docs/`) and code comments, or no documentation updates needed
Contributor
|
@faddat , as discussed yesterday during Tendermint's community call, here is a git command I just ran to gauge the changes between % git fetch; for d in $(echo */); do echo "$d:\t\t$(git diff origin/main..origin/v0.36.x --shortstat -- $d)"; done
DOCKER/: 9 files changed, 18 insertions(+), 123 deletions(-)
abci/: 39 files changed, 6897 insertions(+), 7404 deletions(-)
blocksync/: 8 files changed, 1885 deletions(-)
cmd/: 29 files changed, 1834 insertions(+), 1365 deletions(-)
config/: 5 files changed, 1017 insertions(+), 916 deletions(-)
consensus/: 26 files changed, 14283 deletions(-)
crypto/: 36 files changed, 252 insertions(+), 1012 deletions(-)
docs/: 238 files changed, 4814 insertions(+), 23093 deletions(-)
evidence/: 9 files changed, 2759 deletions(-)
internal/: 305 files changed, 78124 insertions(+), 247 deletions(-)
libs/: 96 files changed, 1085 insertions(+), 11204 deletions(-)
light/: 38 files changed, 6628 insertions(+), 2588 deletions(-)
mempool/: 23 files changed, 4999 deletions(-)
networks/: 26 files changed, 207 insertions(+), 133 deletions(-)
node/: 6 files changed, 1709 insertions(+), 1502 deletions(-)
p2p/: 45 files changed, 13750 deletions(-)
privval/: 25 files changed, 2042 insertions(+), 741 deletions(-)
proto/: 53 files changed, 6839 insertions(+), 3626 deletions(-)
proxy/: 13 files changed, 1496 deletions(-)
rpc/: 73 files changed, 5429 insertions(+), 7491 deletions(-)
scripts/: 20 files changed, 1822 insertions(+), 268 deletions(-)
spec/: 82 files changed, 4500 insertions(+), 2957 deletions(-)
state/: 43 files changed, 9532 deletions(-)
statesync/: 14 files changed, 3634 deletions(-)
store/: 2 files changed, 1234 deletions(-)
test/: 69 files changed, 2303 insertions(+), 2523 deletions(-)
third_party/: 1 file changed, 3 deletions(-)
tools/: 10 files changed, 2 insertions(+), 895 deletions(-)
types/: 53 files changed, 5465 insertions(+), 2396 deletions(-)
version/: 1 file changed, 25 insertions(+), 8 deletions(-)
%This should give a rough idea on where the changes we are likely to cherry-pick concentrate. I propose to draw the line at top-level directories where the sum of insertions + deletions is less than 2000. So the directories that should not apply
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR changes away from gofmt and goimports to gofumpt. This allows the use of the
latest version of golangci-lint and makes the code much easier to read overall.
Since there are clearly issues in linting, a second PR will ensure that CI fails when the
linter fails.
A serious observation is that we should improve on the boring items.
Total changes:
solve conflict when running golangci-lint locally
ensure that golangci-lint runs every time files are changed and matches a local dev env
adhere to PHILOSOPHY.md
gofumpt, a code formatting tool.CHANGELOG_PENDING.mdupdated, or no changelog entry neededdocs/) and code comments, or nodocumentation updates needed