jpictl: fix cloud.yaml unmarshalling #32

Merged
amery merged 2 commits from pr-amery-unmarshaltext into main 11 months ago
amery commented 11 months ago
Owner

fix cloud.yaml unmarshalling by implementing UnmarshalText on wireguard endpoint addresses and keys

fix cloud.yaml unmarshalling by implementing UnmarshalText on wireguard endpoint addresses and keys
amery added 2 commits 11 months ago
634941a1ef wireguard: implement EndpointAddress.UnmarshalText
amery requested review from karasz 11 months ago
karasz approved these changes 11 months ago
karasz left a comment
Owner

Looks a bit strange to me to have switch statement with only 2 cases from which one is default, but otherwise LGTM

Looks a bit strange to me to have `switch` statement with only 2 cases from which one is `default`, but otherwise LGTM
Poster
Owner

Looks a bit strange to me to have switch statement with only 2 cases from which one is default, but otherwise LGTM

I thought you preferred switch over if/else @karasz

> Looks a bit strange to me to have `switch` statement with only 2 cases from which one is `default`, but otherwise LGTM I thought you preferred switch over if/else @karasz
Owner

Looks a bit strange to me to have switch statement with only 2 cases from which one is default, but otherwise LGTM

I thought you preferred switch over if/else @karasz

This would have been an

func....{
if err != nil {
    return err
}

return
}

for anything that has more than 2 branches I prefer switch as it seems to me more appropriate .

> > Looks a bit strange to me to have `switch` statement with only 2 cases from which one is `default`, but otherwise LGTM > > I thought you preferred switch over if/else @karasz This would have been an ```go func....{ if err != nil { return err } return } ``` for anything that has more than 2 branches I prefer `switch` as it seems to me more appropriate .
amery added 2 commits 11 months ago
125a4c0dbe wireguard: implement EndpointAddress.UnmarshalText
Poster
Owner

@karasz amended accordingly, and rebased over the current main

@karasz amended accordingly, and rebased over the current `main`
amery requested review from karasz 11 months ago
karasz approved these changes 11 months ago
karasz left a comment
Owner

+1

+1
amery merged commit 5ef6d45ef7 into main 11 months ago
amery deleted branch pr-amery-unmarshaltext 11 months ago

Reviewers

karasz approved these changes 11 months ago
The pull request has been merged as 5ef6d45ef7.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.