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

fix: remove explicit param from GossipSubParams constructor #1080

Merged
merged 3 commits into from
Apr 9, 2024

Conversation

diegomrsantos
Copy link
Collaborator

@diegomrsantos diegomrsantos commented Apr 4, 2024

I believe explicit is used to check if the GossipSubParams instance was created by the user either passing params to GossipSubParams(...) or GossipSubParams.init(...). In the first case explicit should be set to true when calling the Nim constructor, in the second case, the param isn't necessary and should be always be set to true by init. If none of those options were used, it means the instance was created by using Nim default values. In this case, GossipSubParams.init() should be called to set the values to their default value defined by nim-libp2p.
https://github.com/status-im/nim-libp2p/blob/5e55ca3b69e18a56a0a5a34941676f4df08523ae/libp2p/protocols/pubsub/gossipsub.nim#L774

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.80%. Comparing base (03f67d3) to head (403449d).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           unstable    #1080      +/-   ##
============================================
- Coverage     84.82%   84.80%   -0.02%     
============================================
  Files            91       91              
  Lines         15428    15428              
============================================
- Hits          13087    13084       -3     
- Misses         2341     2344       +3     
Files Coverage Δ
libp2p/protocols/pubsub/gossipsub.nim 86.46% <ø> (ø)

... and 1 file with indirect coverage changes

@diegomrsantos diegomrsantos merged commit 89cad5a into unstable Apr 9, 2024
9 checks passed
@diegomrsantos diegomrsantos deleted the explicit-param branch April 9, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants