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

feat(p2p): refactor p2p config options #861

Merged
merged 9 commits into from
May 16, 2024
Merged

Conversation

srene
Copy link
Contributor

@srene srene commented May 14, 2024

PR Standards

All p2p config parameters have been moved to P2P to specific paramaters, to avoid confusion with Tendermint p2p options

Opening a pull request should be able to meet the following requirements


Close #797

<-- Briefly describe the content of this pull request -->

For Author:

  • Targeted PR against correct branch
  • included the correct type prefix in the PR title
  • Linked to Github issue with discussion and accepted design
  • Targets only one github issue
  • Wrote unit and integration tests
  • All CI checks have passed
  • Added relevant godoc comments

For Reviewer:

  • confirmed the correct type prefix in the PR title
  • Reviewers assigned
  • confirmed all author checklist items have been addressed

After reviewer approval:

  • In case targets main branch, PR should be squashed and merged.
  • In case PR targets a release branch, PR should be rebased.

@srene srene self-assigned this May 14, 2024
@srene srene requested a review from a team as a code owner May 14, 2024 14:16
@srene srene marked this pull request as draft May 14, 2024 14:25
@srene srene marked this pull request as ready for review May 14, 2024 14:28
Copy link

codecov bot commented May 14, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 39.65%. Comparing base (851b312) to head (1913a9e).
Report is 25 commits behind head on main.

❗ Current head 1913a9e differs from pull request most recent head 842a7b7. Consider uploading reports for the commit 842a7b7 to get more accurate results

Files Patch % Lines
config/flags.go 33.33% 8 Missing and 4 partials ⚠️
config/p2p.go 25.00% 4 Missing and 2 partials ⚠️
config/config.go 0.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #861      +/-   ##
==========================================
- Coverage   40.44%   39.65%   -0.80%     
==========================================
  Files         120      118       -2     
  Lines       18935    18703     -232     
==========================================
- Hits         7659     7416     -243     
- Misses      10201    10232      +31     
+ Partials     1075     1055      -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mtsitrin
mtsitrin previously approved these changes May 15, 2024
Copy link
Contributor

@mtsitrin mtsitrin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@danwt danwt left a comment

Choose a reason for hiding this comment

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

I think we should have a better name for BootstrapTime

config/config.go Outdated Show resolved Hide resolved
config/p2p.go Outdated Show resolved Hide resolved
conv/addr.go Show resolved Hide resolved
@srene srene force-pushed the srene/797-change-p2p-options branch from 23a92c0 to 3603b27 Compare May 15, 2024 19:50
@srene srene requested review from mtsitrin and danwt May 15, 2024 19:50
Copy link
Contributor

@danwt danwt left a comment

Choose a reason for hiding this comment

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

gj

@mtsitrin mtsitrin merged commit 70d9460 into main May 16, 2024
6 checks passed
@mtsitrin mtsitrin deleted the srene/797-change-p2p-options branch May 16, 2024 06:48
omritoptix pushed a commit that referenced this pull request May 18, 2024
Co-authored-by: Daniel T <30197399+danwt@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

improve p2p configuration options
3 participants