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.

Sign up for GitHub

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

Jump to bottom

feat: implement Debug trait #4426

Merged
merged 9 commits into from
Sep 6, 2023
Merged

feat: implement Debug trait #4426

merged 9 commits into from
Sep 6, 2023

Conversation

oblique
Copy link
Contributor

@oblique oblique commented Sep 3, 2023 •
edited by thomaseizinger
Loading

Description

Notes & 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

thomaseizinger
thomaseizinger reviewed Sep 3, 2023
View reviewed changes
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.

Agreed with your general view. We need changelog entries and version bumps for all crates that are touched :)

swarm/src/handler.rs Show resolved Hide resolved
swarm/src/handler/one_shot.rs Outdated Show resolved Hide resolved
@thomaseizinger thomaseizinger changed the title feat: Implement missing Debug trait feat: implement Debug trait on events Sep 3, 2023
@thomaseizinger thomaseizinger changed the title feat: implement Debug trait on events feat: implement Debug trait Sep 3, 2023
@oblique
Copy link
Contributor Author

oblique commented Sep 3, 2023

Added changelogs and bump the versions. These are not breaking changes since the where clause where added only on the impl Debug and not on the actual structs.

thomaseizinger
thomaseizinger reviewed Sep 4, 2023
View reviewed changes
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.

Thanks! A few minor comments! :)

core/CHANGELOG.md Outdated Show resolved Hide resolved
swarm/CHANGELOG.md Outdated Show resolved Hide resolved
swarm/CHANGELOG.md Outdated Show resolved Hide resolved
core/CHANGELOG.md Outdated Show resolved Hide resolved
swarm/src/handler.rs Show resolved Hide resolved
swarm/src/handler/one_shot.rs Outdated Show resolved Hide resolved
oblique and others added 5 commits September 4, 2023 11:37
mxinden
mxinden reviewed Sep 4, 2023
View reviewed changes
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Thank you for the work! In case this is urgent for you, I can cut releases once this is merged. Let me know.

@mxinden
Copy link
Member

mxinden commented Sep 4, 2023

Will delay for final review by @thomaseizinger before merging.

@oblique
Copy link
Contributor Author

oblique commented Sep 4, 2023

Thank you for the work! In case this is urgent for you, I can cut releases once this is merged. Let me know.

This is not urgent. Take your time.

thomaseizinger
thomaseizinger previously approved these changes Sep 4, 2023
View reviewed changes
@mergify mergify bot dismissed thomaseizinger’s stale review September 6, 2023 11:36

Approvals have been dismissed because the PR was updated after the send-it label was applied.

thomaseizinger
thomaseizinger previously approved these changes Sep 6, 2023
View reviewed changes
@mergify
Copy link
Contributor

mergify bot commented Sep 6, 2023

This pull request has merge conflicts. Could you please resolve them @oblique? 🙏

@mergify mergify bot dismissed thomaseizinger’s stale review September 6, 2023 14:24

Approvals have been dismissed because the PR was updated after the send-it label was applied.

@mergify mergify bot merged commit 6715281 into libp2p:master Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mxinden mxinden mxinden approved these changes

@thomaseizinger thomaseizinger thomaseizinger left review comments

Assignees
No one assigned
Labels
send-it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@oblique @mxinden @thomaseizinger