Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

run linter in CI #311

Merged
merged 5 commits into from
Oct 16, 2017
Merged

run linter in CI #311

merged 5 commits into from
Oct 16, 2017

Conversation

zramsay
Copy link
Contributor

@zramsay zramsay commented Sep 21, 2017

and address a few fixes

@codecov
Copy link

codecov bot commented Sep 21, 2017

Codecov Report

❗ No coverage uploaded for pull request base (develop@bdad918). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #311   +/-   ##
==========================================
  Coverage           ?   24.91%           
==========================================
  Files              ?       11           
  Lines              ?      578           
  Branches           ?        0           
==========================================
  Hits               ?      144           
  Misses             ?      410           
  Partials           ?       24

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdad918...6ca2477. Read the comment docs.

Copy link
Contributor

@odeke-em odeke-em left a comment

Choose a reason for hiding this comment

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

Thank you @zramsay, LGTM except for one request.

g := new(core.Genesis)
if err := json.Unmarshal(defaultGenesisBlob, g); err != nil {
log.Fatalf("parsing defaultGenesis: %v", err)
}
return g
}()

func bigString(s string) *big.Int {
func bigString() *big.Int {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure we should change this, as it is there for convenience despite not using another string except "10000000000000000000000000000000000"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i just go by the linting suggestions. if you think it's better to // nolint then we can

Choose a reason for hiding this comment

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

Let's keep it how it is and add // nolint.

Copy link

@adrianbrink adrianbrink left a comment

Choose a reason for hiding this comment

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

Just the change with the bigString function. Otherwise LGTM

g := new(core.Genesis)
if err := json.Unmarshal(defaultGenesisBlob, g); err != nil {
log.Fatalf("parsing defaultGenesis: %v", err)
}
return g
}()

func bigString(s string) *big.Int {
func bigString() *big.Int {

Choose a reason for hiding this comment

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

Let's keep it how it is and add // nolint.

@zramsay zramsay force-pushed the fix-linting branch 2 times, most recently from b424c14 to 8a21433 Compare September 25, 2017 13:02
@zramsay
Copy link
Contributor Author

zramsay commented Sep 25, 2017

done

@adrianbrink
Copy link

@zramsay The problem remains, that circle ci does not have enough memory to run the entire linter. Either we have to turn some of these off for now or we really need to upgrade circle.

@zramsay
Copy link
Contributor Author

zramsay commented Sep 26, 2017

weird. I've had no problems running linter in other repos ...

@adrianbrink
Copy link

Ethermint is a really large codebase with its dependencies. It might be something else. @zramsay Can you investigate this and see if you can get CircleCi to pass because I think this PR is super helpful for code quality.

@adrianbrink adrianbrink merged commit 79afe33 into develop Oct 16, 2017
@adrianbrink adrianbrink deleted the fix-linting branch October 16, 2017 09:32
adrianbrink pushed a commit that referenced this pull request Oct 22, 2017
* linter: run it during CI

* Break import cycle in ethereum package

* Disable all linters and enable only unused
adrianbrink pushed a commit that referenced this pull request Oct 22, 2017
* linter: run it during CI

* Break import cycle in ethereum package

* Disable all linters and enable only unused
adrianbrink pushed a commit that referenced this pull request Oct 22, 2017
* linter: run it during CI

* Break import cycle in ethereum package

* Disable all linters and enable only unused
adrianbrink pushed a commit that referenced this pull request Oct 22, 2017
* linter: run it during CI

* Break import cycle in ethereum package

* Disable all linters and enable only unused
adrianbrink pushed a commit that referenced this pull request Oct 22, 2017
* linter: run it during CI

* Break import cycle in ethereum package

* Disable all linters and enable only unused
adrianbrink pushed a commit that referenced this pull request Oct 22, 2017
* linter: run it during CI

* Break import cycle in ethereum package

* Disable all linters and enable only unused
i-norden pushed a commit to vulcanize/old_ethermint that referenced this pull request Aug 31, 2020
* vuepress

* docs: vuepress setup and TODOs

* doc scripts

* update Makefile and gitignore

* more docs updates

* gitignore

* metamask instructions

* update image

* updates

* updates from call

* docs: vuepress config and home.vue (cosmos#350)

* update uncles return (cosmos#337)

* x/evm: fix EndBlock consensus failure (cosmos#334)

* add test for sending tx w/ 21000 gas

* improve rpc transfer test

* use ctx in EndBlock

* UpdateAccounts and ClearStateObjects with passed in context

* log ethereum address on error

Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>

* update Ethermint color variables

* add header and footer logo

* tweak config.js

* WIP custom homepage.vue

* add layout to docs/README

* update color variables

* add eth logo black and white

* tweak docs/README

* update logo and logo-bw svg

* bump 1.0.167

* homepage → home

* add icon-code, icon-rocket

* layout: home, remove configurable frontmatter: label, read, use

* clean up config.js

* bump 1.0.168

* fix missing comma from resolving conflicts

* update sidebar, config nav, path

* remove left whitespace on the header and footer logos

* clean up home.vue, docs/README

* update ethermint forum url in footer.links

* comment out custom true to enable searchbar in subpages

* remove external link icon for Guides

* comments, revert custom true

* clean up config.js, add specifications icon

Co-authored-by: noot <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>

* final touches

Co-authored-by: Cyrus Goh <[email protected]>
Co-authored-by: noot <[email protected]>
faddat pushed a commit to faddat/ethermint that referenced this pull request Aug 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants