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

Add some linting to gaia #188

Merged
merged 6 commits into from
Nov 14, 2019
Merged

Add some linting to gaia #188

merged 6 commits into from
Nov 14, 2019

Conversation

tac0turtle
Copy link
Member

@tac0turtle tac0turtle commented Nov 7, 2019

Signed-off-by: Marko Baricevic marbar3778@yahoo.com

  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • 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 to the Unreleased section in CHANGELOG.md

  • Reviewed 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 PR #XYZ: [title]" (coding standards)

- closes #187

Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
@tac0turtle tac0turtle self-assigned this Nov 7, 2019
lcd_test/lcd_test.go Show resolved Hide resolved
lcd_test/lcd_test.go Show resolved Hide resolved
@tac0turtle tac0turtle marked this pull request as ready for review November 11, 2019 08:10
@tac0turtle tac0turtle requested a review from fedekunze November 11, 2019 08:10
@tac0turtle tac0turtle added the R4R label Nov 11, 2019
Copy link
Collaborator

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

ACK, pending the comment on prealloc

app/export.go Show resolved Hide resolved
@@ -151,8 +151,7 @@ func defaultGenesis(config *tmcfg.Config, nValidators int, initAddrs []sdk.AccAd

// append any additional (non-proposing) validators
var genTxs []auth.StdTx
var genAccounts []authexported.GenesisAccount

genAccounts := make([]authexported.GenesisAccount, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't preallocate to 0, just add nolint

Copy link
Contributor

Choose a reason for hiding this comment

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

even better, you already know the size to allocate: len(initAddrs) + nValidators

Copy link
Member Author

Choose a reason for hiding this comment

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

len(initAddrs) + nValidators this seems to be causing most the tests in lcd_test to fail

Copy link
Contributor

Choose a reason for hiding this comment

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

are you setting values in the array correctly? i.e. genAccounts[genAccsIdx] = genAccount

Copy link
Collaborator

Choose a reason for hiding this comment

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

.golangci.yml Outdated Show resolved Hide resolved
.golangci.yml Outdated Show resolved Hide resolved
app/export.go Show resolved Hide resolved
cli_test/test_helpers.go Show resolved Hide resolved
@@ -151,8 +151,7 @@ func defaultGenesis(config *tmcfg.Config, nValidators int, initAddrs []sdk.AccAd

// append any additional (non-proposing) validators
var genTxs []auth.StdTx
var genAccounts []authexported.GenesisAccount

genAccounts := make([]authexported.GenesisAccount, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

even better, you already know the size to allocate: len(initAddrs) + nValidators

@jackzampolin jackzampolin merged commit 967fc43 into master Nov 14, 2019
@jackzampolin jackzampolin deleted the marko/linting branch November 14, 2019 20:04
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.

Add more golangci linters
5 participants