Skip to content

sail: end-to-end sharing#1499

Merged
nicks merged 1 commit into
masterfrom
nicks/ch2240/sail
Apr 16, 2019
Merged

sail: end-to-end sharing#1499
nicks merged 1 commit into
masterfrom
nicks/ch2240/sail

Conversation

@nicks

@nicks nicks commented Apr 16, 2019

Copy link
Copy Markdown
Contributor

Hello @jazzdan, @landism,

Please review the following commits I made in branch nicks/ch2240/sail:

501c173 (2019-04-16 14:35:10 -0400)
sail: end-to-end sharing

@nicks
nicks requested review from jazzdan and landism April 16, 2019 18:37
@nicks

nicks commented Apr 16, 2019

Copy link
Copy Markdown
Contributor Author

with this pr, we're now at a place where we can replicate the data to the sail server, and view it in a UI served by the sail server. it's still super buggy and needs lots of work, particularly in

  • how we deploy the server
  • how the share interaction works
  • properly setting up and tearing down the connections
  • making the ux work correctly

@nicks
nicks force-pushed the nicks/ch2240/sail branch from 501c173 to 0bfe326 Compare April 16, 2019 18:45

room := NewRoom(conn)
s.rooms[room.id] = room
log.Printf("newRoom: %s", room.id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👀

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are a bunch of these logs: are they meant to be used long term? If so can we make them structured logs so they'd be easy to pick up in Honeycomb?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yep, the logs are intentional. wanted to get something working and debuggable before we start thinking about logging strategy. a lot will need to change before we can even deploy this

Comment thread web/src/HUD.tsx Outdated
}

render() {
debugger

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👀

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

😬

@nicks
nicks force-pushed the nicks/ch2240/sail branch from 0bfe326 to 5c96207 Compare April 16, 2019 19:08
@nicks
nicks merged commit 3b64d01 into master Apr 16, 2019
@nicks
nicks deleted the nicks/ch2240/sail branch April 16, 2019 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants