celestiaorg / celestia-node

Celestia Data Availability Nodes
Apache License 2.0
900 stars 879 forks source link

chore: add dupword CI && remove repetitive words globally #3360

Closed goofylfg closed 3 weeks ago

goofylfg commented 4 weeks ago

use https://github.com/Abirdcfly/dupword to check duplicate words

output:

celestia-node/share/eds/retriever_quadrant.go:71:4: Duplicate words (into) found
celestia-node/share/availability/full/reconstruction_test.go:179:2: Duplicate words (L) found
celestia-node/share/getters/getter_test.go:338:2: Duplicate words (D) found
celestia-node/share/p2p/peers/manager_test.go:86:3: Duplicate words (be) found
ramin commented 3 weeks ago

hi @goofylfg thank you! I thought i had caught all these 😄

Generally, we tend NOT to accept simple grammar/spelling fixes. That said, this project uses golangci-lint and has a CI integration to run it. It also appears dupword can be enabled https://golangci-lint.run/usage/linters/#dupword.

It appears it would be fairly trivial to enable dupword in our config, and modify this PR so the contribution is enabling dupword in our linter, then have the fixes accompany that as a change. This is something we'd appreciate and accept.

Would you be interested in giving it a go?

goofylfg commented 3 weeks ago

hi @goofylfg thank you! I thought i had caught all these 😄

Generally, we tend NOT to accept simple grammar/spelling fixes. That said, this project uses golangci-lint and has a CI integration to run it. It also appears dupword can be enabled https://golangci-lint.run/usage/linters/#dupword.

It appears it would be fairly trivial to enable dupword in our config, and modify this PR so the contribution is enabling dupword in our linter, then have the fixes accompany that as a change. This is something we'd appreciate and accept.

Would you be interested in giving it a go?

sure, I will add dupword CI integration.

goofylfg commented 3 weeks ago
celestia-node/share/eds/retriever_quadrant.go:71:4: Duplicate words (into) found
celestia-node/share/availability/full/reconstruction_test.go:179:2: Duplicate words (L) found
celestia-node/share/getters/getter_test.go:338:2: Duplicate words (D) found
celestia-node/share/p2p/peers/manager_test.go:86:3: Duplicate words (be) found

"L","D" is false positve, so I added it to ignore list. @ramin plz check it ~

ramin commented 3 weeks ago

@goofylfg oh awesome nice job! My only request would be to do //nolint:dupword for those _test.go files (either whole file, or the area with the false positive) vs adding to config for single letters like that, then IMO it's gtg

goofylfg commented 3 weeks ago

@goofylfg oh awesome nice job! My only request would be to do //nolint:dupword for those _test.go files (either whole file, or the area with the false positive) vs adding to config for single letters like that, then IMO it's gtg

fixed, thks for the feedback @ramin

codecov-commenter commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 44.64%. Comparing base (2469e7a) to head (d1b0251). Report is 60 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3360 +/- ## ========================================== - Coverage 44.83% 44.64% -0.20% ========================================== Files 265 272 +7 Lines 14620 15244 +624 ========================================== + Hits 6555 6805 +250 - Misses 7313 7651 +338 - Partials 752 788 +36 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.