-
Notifications
You must be signed in to change notification settings - Fork 18k
net: add text marshalling and unmarshalling for HardwareAddr #34452
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
Conversation
This PR (HEAD: 85a3e00) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/196817 to see it. Tip: You can toggle comments from me using the |
Message from Gobot Gobot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
Message from Akhil Indurti: Patch Set 1: Code-Review-1 Hey Kurtis, thank you for submitting this! I think we should wait for the issue to resolve first w.r.t the backwards compatibility concerns. Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
This PR (HEAD: b18832e) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/196817 to see it. Tip: You can toggle comments from me using the |
Message from Kurtis Nusbaum: Patch Set 2:
Hey Ankhil, Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
The HardwareAddr type has the ability to be transformed to and from a string. However, this capability is not exposed in a manner suitable for use with various forms of marshalling and unmarshaling of text (e.g. JSON or YAML). Let's add the proper functions so that HardwareAddr implements the TextMarshaller and TextUnmarshaler interfaces from the encoding package. Fixes golang#29678
…ompatibility reasons
- Create `matchErr` function and simplify result testing in `TestHardwareAddr_UnmarshalText` - Fix Lbrace in `TestHardwareAddr_UnmarshalText` - Change `Fatalf`s to just `Fatal`s in `TestHardwareAddr_MarshalText`
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Message from Akhil Indurti: Patch Set 2: (7 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
This PR (HEAD: df49700) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/196817 to see it. Tip: You can toggle comments from me using the |
This PR (HEAD: 9e07592) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/196817 to see it. Tip: You can toggle comments from me using the |
Message from Akhil Indurti: Patch Set 4: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
This PR (HEAD: 5660642) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/196817 to see it. Tip: You can toggle comments from me using the |
Message from Akhil Indurti: Patch Set 5: Code-Review+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
07b4abd
to
19a7490
Compare
Message from Akhil Indurti: Patch Set 5: Run-TryBot+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
Message from Gobot Gobot: Patch Set 5: TryBots beginning. Status page: https://farmer.golang.org/try?commit=2afdb754 Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
Message from Gobot Gobot: Patch Set 5: Build is still in progress... Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed. Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
Message from Akhil Indurti: Patch Set 5: -Run-TryBot -Code-Review Seems like the base64 import violates the policy defined in go/build/deps_test.go. Is it absolutely needed for Unmarshal? If so, this needs to be discussed in the issue. Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
Message from Kurtis Nusbaum: Patch Set 5:
Hey Akhil, This actually was discussed on the issue. The claim was that in order to be truly backwards compatible with pervious versions of Go, we needed to be able to decode the potentially base64 encoded byte arrays that theoretically may be have serialized in current and previous versions of Go. I don't know of any other way to do that besides using the base64 package (or copy and pasting from the base64 package). It seems we're stuck between two requirements here. Which would you prefer I go with? Or is there a third option? Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
4a7ed1f
to
0f992b9
Compare
Message from Go Bot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
Message from Go Bot: Patch Set 5: TryBots beginning. Status page: https://farmer.golang.org/try?commit=2afdb754 Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
Message from Go Bot: Patch Set 5: Build is still in progress... Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed. Please don’t reply on this GitHub thread. Visit golang.org/cl/196817. |
The HardwareAddr type has the ability to be transformed to and from a
string. However, this capability is not exposed in a manner suitable
for use with various forms of marshalling and unmarshaling of text
(e.g. JSON or YAML). Let's add the proper functions so that
HardwareAddr implements the TextMarshaller and TextUnmarshaler
interfaces from the encoding package.
Fixes #29678