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

Change GET query parameters to be more compact #19

Merged
merged 2 commits into from Nov 12, 2017
Merged

Change GET query parameters to be more compact #19

merged 2 commits into from Nov 12, 2017

Conversation

mcmanus
Copy link
Member

@mcmanus mcmanus commented Nov 6, 2017

closes issue #7

Copy link

@roryhewitt roryhewitt left a comment

Choose a reason for hiding this comment

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

I like the change to 'ct' for Content-Type, but I think 'body' is important enough where reducing it by only two bytes to 'rb' isn't worth the change. Perhaps there could be an abbreviated 'ct' value - e.g. 'ct=du'? Or, if 'ct' is not specified, then 'application/dns-udpwireformat' iMUST be assumed by the DNS server?

@mcmanus
Copy link
Member Author

mcmanus commented Nov 7, 2017

I could go either way on body. You're right that the actual mime type is a bit of a problem.. I'll try it with a ct name argument without a value having the default meaning. I think that's explicit enough, while I don't really like totally implicit things like the absence of ct (what if you mis-spell it?)

@roryhewitt
Copy link

@mcmanus Actually what was the reasoning behind passing Content-Type as a query parameter instead of as the Content-Type header? Presumably any DNS API server must be able to parse headers...

allow empty ct to indicate default udp wireformat content-type
@mcmanus
Copy link
Member Author

mcmanus commented Nov 7, 2017

Content-Type as a header refers to the message payload which isn't what we're trying to describe here.. (its used on POST, where it matches..)

@mcmanus mcmanus changed the title Change GET query names to shorter ct and rb Change GET query parameters to be more compact Nov 7, 2017
@mcmanus mcmanus merged commit fb31cda into master Nov 12, 2017
@paulehoffman paulehoffman deleted the issue7 branch February 2, 2018 17:47
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.

None yet

2 participants