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

Sort headers before printing #16

Closed
wants to merge 1 commit into from
Closed

Sort headers before printing #16

wants to merge 1 commit into from

Conversation

mibk
Copy link
Contributor

@mibk mibk commented Sep 24, 2016

No description provided.

@davecheney
Copy link
Owner

davecheney commented Sep 24, 2016

I thought about this previously and have two suggestions.

  1. http headers are not sorted, they probably get unsorted more in this version compared to the python version because of the way maps work in Go vs whatever curl is doing.
  2. if we are going to sort headers, it shouldn't just be plain alpha sort, it should

I know this is more work, but I think this is only worth doing if it's done to this degree of precision.

@davecheney
Copy link
Owner

Also, please read the contributing section about slinging code before discussing a feature. I would like it if you took my comment #16 (comment) and created a new issue where we can discuss the implementation before jumping to the code.

@mibk
Copy link
Contributor Author

mibk commented Sep 24, 2016

Apologies, this is my mistake. I have read the contributing section but thought this is a small and reasonable enough change it's not worth creating an issue for it. Well, lesson learnt. Sorry again.

Your suggestion seems reasonable. I think I'm able to do it. But now I should probably go to bed...

@davecheney
Copy link
Owner

This is my fault, I should have made it clear in the to-do that sorting
isn't implemented because I wanted something more than just an alpha sort.

On Sat, 24 Sep 2016, 10:31 Michal Bohuslávek notifications@github.com
wrote:

Apologies, this is my mistake. I have read the contributing section but
thought this is a small and reasonable enough change it's not worth
creating an issue for it. Well, lesson learnt. Sorry again.

Your suggestion seems reasonable. I think I'm able to do it. But now I
should probably go to bed...


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#16 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAcA9miog-B3zXrlNj3ogkuWgGkNbuEks5qtG9vgaJpZM4KFf7f
.

This was referenced Sep 24, 2016
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