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

Remove manual nat manager #1041

Merged
merged 8 commits into from
Jun 5, 2020

Conversation

matkt
Copy link
Contributor

@matkt matkt commented Jun 4, 2020

Signed-off-by: Karim TAAM karim.t2am@gmail.com

PR description

We decided to add a manual mode during the implementation of the NatManager. This mode does not seem really useful because it does the same thing as a NONE mode (it takes the parameters p2p-host, p2p-port, etc). At the moment this mode does not bring much and seems to be confusing. To avoid confusion the best is to delete ManualNatManager

This requires an update of the documentation (https://besu.hyperledger.org/en/stable/HowTo/Find-and-Connect/Specifying-NAT/#manual)

  • Remove the MANUAL part
  • Change NONE part
By specifying NONE you can set the following parameters if needed:
--p2p-host and --p2p-port define the advertised host and port for the P2P service.
--rpc-http-host and rpc-http-port define the advertised host and port for the JSON-RPC service.

matkt added 2 commits May 13, 2020 22:55
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
@matkt matkt added the breaking This can only be addressed/merged for a release that allows user-facing changes to be breaking. label Jun 4, 2020
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
@matkt matkt marked this pull request as ready for review June 4, 2020 09:43
Copy link
Contributor

@AbdelStark AbdelStark 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 to me.

Copy link
Contributor

@RatanRSur RatanRSur left a comment

Choose a reason for hiding this comment

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

I forgot to do this in my PR so just reminding you here to write an entry in the changelog in this PR :) It will help out the docs team so they don't have to do this last minute when the release is happening.

matkt added 2 commits June 5, 2020 13:32
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
@matkt
Copy link
Contributor Author

matkt commented Jun 5, 2020

I forgot to do this in my PR so just reminding you here to write an entry in the changelog in this PR :) It will help out the docs team so they don't have to do this last minute when the release is happening.

I added it in the Upcoming 1.5 release section of the CHANGELOG. feel free to tell me if it's not ok

@matkt matkt added the doc-change-required Indicates an issue or PR that requires doc to be updated label Jun 5, 2020
@matkt matkt linked an issue Jun 5, 2020 that may be closed by this pull request
CHANGELOG.md Outdated Show resolved Hide resolved
matkt added 2 commits June 5, 2020 16:39
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
@matkt matkt merged commit 7371d56 into hyperledger:master Jun 5, 2020
@matkt matkt deleted the feature/remove-manual-nat-manager branch June 18, 2020 09:48
@MadelineMurray
Copy link
Contributor

@bgravenorst bgravenorst removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This can only be addressed/merged for a release that allows user-facing changes to be breaking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Manual NAT manager
6 participants