-
Notifications
You must be signed in to change notification settings - Fork 10
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
Rework face API and fix transports #67
Conversation
Done with this, not doing ethernet at the moment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for now
time.Sleep(10 * time.Second) | ||
|
||
// Iterate the face table | ||
t.faces.Range(func(_, face interface{}) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Not related) Really want to see generics supported in sync
package, but it seems to still have a long way to go: golang/go#47657
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is very annoying
I may not have time to do a test run this weekends so I will not initiate merging. Varun, please merge it yourself after test and when you think it is ready. |
No worries, thanks again for the review :D |
Rework face API and fix transports
Replace #66