Skip to content
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

Specify use of mDNS, QUIC, and CBOR #119

Merged
merged 55 commits into from
Dec 7, 2018
Merged

Specify use of mDNS, QUIC, and CBOR #119

merged 55 commits into from
Dec 7, 2018

Conversation

pthatcherg
Copy link
Contributor

No description provided.

Copy link
Member

@tidoust tidoust left a comment

Choose a reason for hiding this comment

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

Content looks good! See inline comments for typos.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
tidoust and others added 10 commits November 12, 2018 12:44
Co-Authored-By: pthatcherg <pthatcherg@users.noreply.github.com>
Co-Authored-By: pthatcherg <pthatcherg@users.noreply.github.com>
Co-Authored-By: pthatcherg <pthatcherg@users.noreply.github.com>
Co-Authored-By: pthatcherg <pthatcherg@users.noreply.github.com>
Co-Authored-By: pthatcherg <pthatcherg@users.noreply.github.com>
Co-Authored-By: pthatcherg <pthatcherg@users.noreply.github.com>
Co-Authored-By: pthatcherg <pthatcherg@users.noreply.github.com>
Co-Authored-By: pthatcherg <pthatcherg@users.noreply.github.com>
Co-Authored-By: pthatcherg <pthatcherg@users.noreply.github.com>
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@btolsch
Copy link
Contributor

btolsch commented Nov 15, 2018

Wow, so apparently making everything a "single comment" had disastrous consequences. I'm new to reviewing on GitHub, so sorry for the spam!

Co-Authored-By: pthatcherg <pthatcherg@users.noreply.github.com>
@pthatcherg
Copy link
Contributor Author

I'll install bikeshed and run it tomorrow.

In the meantime, there are a few things remaining for discussion but I think I've resolved most of them.

@pthatcherg
Copy link
Contributor Author

This conversation/review has gotten complex enough that I'm having trouble tracking all of the things going on.

I think that we're close enough to being in a good state that we should merge the PR and then resolve anything unresolved with issues and or separate PRs.

What do you think?

Copy link
Contributor

@markafoltz markafoltz left a comment

Choose a reason for hiding this comment

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

Looks good with a couple of typos to fix

index.bs Outdated Show resolved Hide resolved
index.bs Outdated
Listening agents act as QUIC clients. Advertising agents act as QUIC servers.

If a listening agent wishes to receive messages from an advertising agent or an
advertising agent wishes to send message to a listening agent, it may wish to
Copy link
Contributor

Choose a reason for hiding this comment

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

s/message/messages/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

index.bs Show resolved Hide resolved
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.

5 participants