Skip to content

Conversation

hugoduncan
Copy link
Contributor

Allow use of EntityTag in other headers that use entity tags.

@GitCop
Copy link

GitCop commented Feb 6, 2015

There were the following issues with your Pull Request

  • Commit: 7fef4c4
    • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md


This message was auto-generated by https://gitcop.com

@hugoduncan hugoduncan force-pushed the feature/factor-entity-tag branch from 7fef4c4 to cbfb58b Compare February 6, 2015 20:36
/// The opaque string in between the DQUOTEs
pub tag: String
}
pub struct Etag(pub EntityTag);
Copy link
Member

Choose a reason for hiding this comment

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

With a newtype like this, could you also add deref!(Etag => EntityTag)?

@seanmonstar
Copy link
Member

What's another example of a header needing an entity tag?

@jdm
Copy link

jdm commented Feb 6, 2015

If-Match and If-None-Match.

@seanmonstar
Copy link
Member

Fair enough! I'm sold. Just the deref nit and I'll merge.

On Fri, Feb 6, 2015, 2:27 PM Josh Matthews notifications@github.com wrote:

If-Match?


Reply to this email directly or view it on GitHub
#296 (comment).

@hugoduncan
Copy link
Contributor Author

Would you like these squashed to a single commit?

@seanmonstar
Copy link
Member

Yes please!

Allow use of EntityTag in other headers that use entity tags.

BREAKING CHANGE: for any consumers of the Etag header, since the entity
tag is now in a tuple.
@hugoduncan hugoduncan force-pushed the feature/factor-entity-tag branch from c4744eb to 28fd5c8 Compare February 7, 2015 00:48
seanmonstar added a commit that referenced this pull request Feb 7, 2015
@seanmonstar seanmonstar merged commit 6aa6605 into hyperium:master Feb 7, 2015
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.

4 participants