Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[R4R] Require moniker on gaiad init #2773

Merged
merged 7 commits into from
Nov 13, 2018
Merged

Conversation

alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Nov 12, 2018

closes: #2757


  • 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 entries in PENDING.md with issue #

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

@alessio
Copy link
Contributor

alessio commented Nov 12, 2018

I'd rather make the flag required. It is a mandatory user input.

@alexanderbez
Copy link
Contributor Author

@alessio I was leaning this way too. Ok 👌

@codecov
Copy link

codecov bot commented Nov 12, 2018

Codecov Report

Merging #2773 into develop will increase coverage by 0.02%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #2773      +/-   ##
===========================================
+ Coverage    56.63%   56.65%   +0.02%     
===========================================
  Files          156      156              
  Lines         9783     9783              
===========================================
+ Hits          5541     5543       +2     
+ Misses        3864     3863       -1     
+ Partials       378      377       -1

@alexanderbez alexanderbez changed the title Generate random moniker on init when not provided Require moniker on gaiad init Nov 12, 2018
@alexanderbez alexanderbez changed the title Require moniker on gaiad init [R4R] Require moniker on gaiad init Nov 12, 2018
Copy link
Member

@jackzampolin jackzampolin left a comment

Choose a reason for hiding this comment

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

So I was able to run tests successfully here (make test and make test_sim_gaia_multi_seed), but when running gaiad init with the built binary it doesn't require the flag and the command runs w/o requiring the flag.

@alexanderbez
Copy link
Contributor Author

@jackzampolin fixed!

Copy link
Member

@jackzampolin jackzampolin left a comment

Choose a reason for hiding this comment

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

tACK

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

TestedACK

@cwgoes cwgoes merged commit a4e5d8e into develop Nov 13, 2018
@cwgoes cwgoes deleted the bez/require-moniker-init branch November 13, 2018 12:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cannot connect to peers on gaia-9001
4 participants