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

Added Types methods to Protocol #315

Merged
merged 4 commits into from
Oct 20, 2023
Merged

Added Types methods to Protocol #315

merged 4 commits into from
Oct 20, 2023

Conversation

elia-bracci-hs
Copy link
Contributor

@elia-bracci-hs elia-bracci-hs commented Oct 20, 2023 •
edited
Loading

Added method Types to return Protocol Types

@elia-bracci-hs elia-bracci-hs marked this pull request as draft October 20, 2023 13:43
nrwiersma
nrwiersma previously approved these changes Oct 20, 2023
View reviewed changes
Copy link
Member

@nrwiersma nrwiersma left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@elia-bracci-hs elia-bracci-hs marked this pull request as ready for review October 20, 2023 14:33
@elia-bracci-hs elia-bracci-hs changed the title Added Types method to return Protocol Types Added Types and OriginalString methods to Protocol Oct 20, 2023
nrwiersma
nrwiersma requested changes Oct 20, 2023
View reviewed changes
protocol.go Outdated Show resolved Hide resolved
@elia-bracci-hs
Copy link
Contributor Author

@nrwiersma thank you for your review 🙏
However, I would like to add also another field in Protocol struct.

I called it OriginalString but we can change it. The intent is to save the string of the protocol as is, using its non-canonical form. WDYT?

I need this to generate Protocol hash

@elia-bracci-hs elia-bracci-hs changed the title Added Types and OriginalString methods to Protocol Added Types methods to Protocol Oct 20, 2023
@nrwiersma
Copy link
Member

You can open an issue or discussion about OriginalString.

@nrwiersma nrwiersma merged commit 50a7897 into hamba:main Oct 20, 2023
2 checks passed
@elia-bracci-hs
Copy link
Contributor Author

elia-bracci-hs commented Oct 23, 2023 •
edited
Loading

@nrwiersma when will it be released? Thank you in advance

@nrwiersma
Copy link
Member

I usually wait a coupe days for any other changes that come in, to group the release. It seems no more will come in, so in a moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nrwiersma nrwiersma nrwiersma approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@elia-bracci-hs @nrwiersma