Skip to content

Remove Headers in favor of Metadata#822

Merged
carl-mastrangelo merged 1 commit intogrpc:masterfrom
carl-mastrangelo:guillotine
Aug 24, 2015
Merged

Remove Headers in favor of Metadata#822
carl-mastrangelo merged 1 commit intogrpc:masterfrom
carl-mastrangelo:guillotine

Conversation

@carl-mastrangelo
Copy link
Contributor

Fixes #649

Copy link
Member

Choose a reason for hiding this comment

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

Just delete, we have no need for path, at least not yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ejona This is a behavior change and thus an API breakage. (Client provided headers will override the method name). Are you sure you want this?

Choose a reason for hiding this comment

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

+1 ... delete it! quick! :)

Copy link
Member

Choose a reason for hiding this comment

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

@carl-mastrangelo, are you thinking we should keep it, or preferring to delete it?

@nmittler
Copy link

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

Hmm... so colon in the name should probably be prevented. I guess this shows we need an issue similar to #725 but for metadata keys. Created #824

@carl-mastrangelo
Copy link
Contributor Author

Friendly ping

@ejona86
Copy link
Member

ejona86 commented Aug 20, 2015

The rebase means I don't know what changed. I'll have to do a full new review.

Copy link
Member

Choose a reason for hiding this comment

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

Remove authority here?

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.

@carl-mastrangelo
Copy link
Contributor Author

@ejona86 PTAL

@ejona86
Copy link
Member

ejona86 commented Aug 24, 2015

@carl-mastrangelo LGTM

@carl-mastrangelo carl-mastrangelo merged commit a508c1d into grpc:master Aug 24, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants