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

Add support for user-provided route tags. #316

Merged
merged 3 commits into from
May 14, 2019
Merged

Add support for user-provided route tags. #316

merged 3 commits into from
May 14, 2019

Conversation

danxmoran
Copy link
Contributor

Fixes #314.

I went with @@ for the syntax to get things rolling, but I'm not tied to it. I plan to have a follow-up PR that adds alphanumeric aliases for it and the other existing methods as part of #315.

Copy link
Member

@zarthross zarthross left a comment

Choose a reason for hiding this comment

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

My initial thoughts are
1.) It would be nice to be able to do a single tag easily as well. (tag @@ route rather than List(tag) @@ route )
2.) Have some examples put into the rho-examples project

@zarthross
Copy link
Member

I'll think on the symbolic operator as well. I have some time tomorrow night, I'll give this a good review then. Thanks for the PR!

Copy link
Member

@zarthross zarthross 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 to me. I think the @@ has grown on me, since shapeless uses @@ for type tags. (I don't think it conflicts because its a type in shapeless and a method here)

My only complaint is the issue with precedence "tag" @@ ("foo" ** GET / "bar") it would be nice to be able to drop the parans for either order of tags vs desc.

@zarthross
Copy link
Member

@danxmoran If you good with the current PR I'll happily merge it in.

@danxmoran
Copy link
Contributor Author

Agreed that the mismatched operator precedence is annoying, but I'm ok with having it merged as-is 👍 thanks for the review!

@danxmoran
Copy link
Contributor Author

@zarthross sorry to bother, is there anything else I can do to help get this merged?

@zarthross
Copy link
Member

@danxmoran I was waiting for some other maintainers to comment, but seeing how no one has... i'll just merge it anyways. It'll get released with our next milestone.

@zarthross zarthross merged commit 4151db0 into http4s:master May 14, 2019
@danxmoran danxmoran deleted the danxmoran-operation-tags-314 branch May 19, 2019 20:23
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.

Feature request: Allow customizing tags for generated operations
2 participants