jpictl: fix cloud.yaml unmarshalling #32

Merged
amery merged 2 commits from pr-amery-unmarshaltext into main 8 months ago
amery commented 8 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 8 months ago
634941a1ef wireguard: implement EndpointAddress.UnmarshalText
amery requested review from karasz 8 months ago
karasz approved these changes 8 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 8 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 8 months ago
karasz approved these changes 8 months ago
karasz left a comment
Owner

+1

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

Reviewers

karasz approved these changes 8 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.