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

swarm-derive/: Remove support for ignoring fields on struct #2842

Merged
merged 3 commits into from Aug 29, 2022

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Aug 24, 2022

Description

With the removal of NetworkBehaviourEventProcess there is no more need for
ignoring fields.

Any objections? Anyone can still see a use-case for fields that need to be ignored?

Links to any relevant issues

Deprecation of NetworkBehaviourEventProcess #2840 and removal #2840.

Past discussions on the removal of poll #2751 (comment) and #2819 (comment)

Open Questions

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

With the removal of `NetworkBehaviourEventProcess` there is no more need for
ignoring fields.
Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

No objections!

@mxinden mxinden merged commit 6855ab9 into libp2p:master Aug 29, 2022
@dariusc93
Copy link
Contributor

If im reading this right, we can have different fields in the struct without it being specific to any behaviour since originally those fields that didnt implement such behaviour would need to be ignored to be there?

@mxinden
Copy link
Member Author

mxinden commented Aug 29, 2022

I am not sure I am following @dariusc93. With this change you can no longer have any fields on your main struct that do not implement NetworkBehaviour. Do you foresee any issues with that?

@dariusc93
Copy link
Contributor

Sorry I wasnt clear (misread the pr when looking over it last night). Ignoring fields do have its benefits. In one of my use case would be to have the struct contain a registry to track QueryId and check against that when receiving a KademliaEvent::OutboundQueryCompleted event and return the results. I know rust-ipfs does the same (though might not be a good example since it havent seen much activities on that side). I might have some workarounds though.

@mxinden
Copy link
Member Author

mxinden commented Aug 30, 2022

Ignoring fields do have its benefits. In one of my use case would be to have the struct contain a registry to track QueryId and check against that when receiving a KademliaEvent::OutboundQueryCompleted event and return the results.

Why not store the "registry to track QueryId" in the object (e.g. struct) that is polling the Swarm?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants