sail: create new room via http endpoint that returns room id and secret (for later surfacing room id in UI) [ch2294]#1528
Conversation
| "github.com/windmilleng/tilt/internal/hud/webview" | ||
| "github.com/windmilleng/tilt/internal/logger" | ||
| "github.com/windmilleng/tilt/internal/model" | ||
| sailCommon "github.com/windmilleng/tilt/internal/sail/common" |
There was a problem hiding this comment.
this seemed clearer than just referring to it as "common" in the code (and more go-like than naming the package sail_common), but idk if this stylistic 🤷♀️
| return roomInfo.RoomID, roomInfo.Secret, nil | ||
| } | ||
|
|
||
| func ProvideSailRoomer(addr model.SailURL) SailRoomer { |
There was a problem hiding this comment.
it's maybe a liiittle weird that this address lives in both the roomer and the client itself? I'm not super worried but wanted to call it out.
| @@ -0,0 +1,11 @@ | |||
| package common | |||
There was a problem hiding this comment.
Why not call this model instead of common? Common doesn't tell me much about what's in this package.
There was a problem hiding this comment.
...damnit i should just put this in model/sail.go shouldn't i? -.-
| } | ||
|
|
||
| r.HandleFunc("/share", s.startRoom) | ||
| r.HandleFunc("/new_room", s.newRoom) |
There was a problem hiding this comment.
Do you think it would be more REST-ful to make the URI /room? And then you could signal that you're want to create a new one by POST'ing to it?
Not sure how much we care about being RESTful though.
|
|
||
| room, err := s.getRoomWithAuth(RoomID(roomID), secret) | ||
| if err != nil { | ||
| log.Printf("connectRoom: %v", err) |
There was a problem hiding this comment.
Do we want to set HTTP 403 here?
There was a problem hiding this comment.
...I don't care enough about parsing the errors to know if it should be 403 or 404 tho, so imma leave a todo
2 participants
Hello @landism, @dmiller,
Please review the following commits I made in branch maiamcc/http-new-room:
027e857 (2019-04-23 15:44:29 -0400)
server test
7415926 (2019-04-23 15:29:55 -0400)
count number of NewRoom calls
9a47562 (2019-04-23 15:24:41 -0400)
fake roomer & green tests
1a2066f (2019-04-23 15:07:40 -0400)
better test err msg
b8abf91 (2019-04-23 15:05:51 -0400)
update make wire to include sail
def7082 (2019-04-23 12:31:09 -0400)
clean up
333f08f (2019-04-22 18:02:47 -0400)
send room id and secret with connect request
8bcc735 (2019-04-22 17:26:14 -0400)
get room id and secret from response
afa2cda (2019-04-22 17:26:09 -0400)
client calls new_room
5ae1dbe (2019-04-22 17:26:04 -0400)
new_room endpoint
Code review reminders, by giving a LGTM you attest that: