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

service/header: add network name to protocol ID #496

Merged
merged 3 commits into from
Mar 8, 2022

Conversation

toanngosy
Copy link
Contributor

This pull request would close #493.

Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

LGTM and thanks for the contribution @toanngosy !

@Wondertan is this the format you were thinking?

We also need to add a changelog entry for this.

@toanngosy
Copy link
Contributor Author

toanngosy commented Mar 6, 2022

Thanks @renaynay ! I added the change into CHANGELOG-PENDING.md.

@toanngosy toanngosy changed the title add network name to protocol ID service/header: add network name to protocol ID Mar 6, 2022
Copy link
Member

@Wondertan Wondertan 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 tackling this.

service/header/p2p_exchange.go Outdated Show resolved Hide resolved
@toanngosy toanngosy requested a review from Wondertan March 7, 2022 15:29
@Wondertan
Copy link
Member

@toanngosy, we did some chore changes on the master that cause fails here, pls do rebase

@toanngosy
Copy link
Contributor Author

@Wondertan it should work now 😄

@Wondertan
Copy link
Member

Pls look at the commit history, @toanngosy. There are multiple duplicates from the master. Seems like artifacts from the rebasr. Needs to be fixed

@toanngosy toanngosy force-pushed the service_header branch 2 times, most recently from b2e2fb0 to ac38780 Compare March 8, 2022 06:32
@toanngosy
Copy link
Contributor Author

sorry, I fixed it. It is now up to date with the main. @Wondertan

Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Member

@renaynay renaynay 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 so much for your contribution @toanngosy ❤️

@Wondertan Wondertan merged commit c3b040a into celestiaorg:main Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

service/header: exchangeProtocolID should include network name from params
3 participants