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

Removed --private option from aleth #5538

Merged
merged 3 commits into from
Apr 1, 2019
Merged

Removed --private option from aleth #5538

merged 3 commits into from
Apr 1, 2019

Conversation

twinstar26
Copy link
Contributor

@twinstar26 twinstar26 commented Mar 31, 2019

Issue Name: Remove --private option from aleth #5531
--private option from aleth has been removed.

@codecov-io
Copy link

codecov-io commented Mar 31, 2019

Codecov Report

Merging #5538 into master will increase coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5538      +/-   ##
==========================================
+ Coverage   61.88%   61.91%   +0.02%     
==========================================
  Files         344      344              
  Lines       28769    28757      -12     
  Branches     3269     3267       -2     
==========================================
+ Hits        17805    17806       +1     
+ Misses       9795     9783      -12     
+ Partials     1169     1168       -1

@halfalicious halfalicious requested review from halfalicious, gumb0 and chfast and removed request for gumb0 March 31, 2019 21:37
@halfalicious
Copy link
Contributor

Can you please update your PR description to mention the issue that you're fixing (and please be sure to include a "#" before the issue ID so that the issue is automatically updated with a link to this PR)?

@halfalicious
Copy link
Contributor

Can you also please remove the definition for enableDiscovery and remove this line in main where it's set to false:

aleth/aleth/main.cpp

Lines 378 to 384 in 4f638b0

if (vm.count("test"))
{
testingMode = true;
enableDiscovery = false;
disableDiscovery = true;
bootstrap = false;
}

Also please replace the check for enableDiscovery here with a check for !disableDiscovery:
if (bootstrap || !remoteHost.empty() || enableDiscovery || listenSet || !preferredNodes.empty())

Copy link
Contributor

@halfalicious halfalicious left a comment

Choose a reason for hiding this comment

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

Looks good overall, just some minor changes

aleth/main.cpp Show resolved Hide resolved
aleth/main.cpp Outdated Show resolved Hide resolved
@twinstar26 twinstar26 changed the title Removed --private option from aleth Remove --private option from aleth #5531 Apr 1, 2019
@twinstar26 twinstar26 changed the title Remove --private option from aleth #5531 Removed --private option from aleth Apr 1, 2019
removed BadPrivateOption from enum AlethErrors as --private option has been removed from aleth
@halfalicious halfalicious merged commit e52571a into ethereum:master Apr 1, 2019
@@ -756,7 +736,7 @@ int main(int argc, char** argv)
};

auto netPrefs = publicIP.empty() ? NetworkConfig(listenIP, listenPort, upnp) : NetworkConfig(publicIP, listenIP ,listenPort, upnp);
netPrefs.discovery = (privateChain.empty() && !disableDiscovery) || enableDiscovery;
netPrefs.discovery = !disableDiscovery;
Copy link
Member

Choose a reason for hiding this comment

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

How do the options for enable/disable discovery look like now?

halfalicious added a commit that referenced this pull request Apr 2, 2019
PR 5538 removed the --private option from aleth
halfalicious added a commit that referenced this pull request Apr 2, 2019
PR 5538 removed the --private option from aleth
halfalicious added a commit that referenced this pull request Apr 3, 2019
* Fix changelog bug and add entry for PR #5538 (PR 5538 removed the --private option from aleth)
* Grouped entries by kind and changed "added" to "removed" for PR which
removed a command-line argument.
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.

4 participants