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

Rewrite #[derive(Queryable)] in derives2 #1529

Merged
merged 1 commit into from
Feb 3, 2018
Merged

Conversation

sgrif
Copy link
Member

@sgrif sgrif commented Feb 2, 2018

This was fairly recently rewritten, so it should in theory be the most
straightforward derive to port. Unfortunately, due to
rust-lang/rust#47311, it's obnoxiously hard to
actually construct a struct in a derive right now. We have to do hacky
workarounds until that is fixed.

@sgrif sgrif requested a review from a team February 2, 2018 23:03
This was fairly recently rewritten, so it should in theory be the most
straightforward derive to port. Unfortunately, due to
rust-lang/rust#47311, it's obnoxiously hard to
actually construct a struct in a derive right now. We have to do hacky
workarounds until that is fixed.
sgrif added a commit that referenced this pull request Feb 3, 2018
Since `QueryableByName` is one of the more recently written derives, it
should have been a really straightforward port. Unfortunately, the
tests for this derive hit multiple rustc bugs

- rust-lang/rust#47983
- rust-lang/rust#47311

I love what we were able to do with the error message here. We could
even go so far as to have the `help` lines point at the struct itself
for the `table_name` annotation if we want to.

I also much prefer the workaround for
rust-lang/rust#47311 in this PR to the one I
did in #1529. I'll need to update that PR if this is merged first.
sgrif added a commit that referenced this pull request Feb 3, 2018
Since `QueryableByName` is one of the more recently written derives, it
should have been a really straightforward port. Unfortunately, the
tests for this derive hit multiple rustc bugs

- rust-lang/rust#47983
- rust-lang/rust#47311

I love what we were able to do with the error message here. We could
even go so far as to have the `help` lines point at the struct itself
for the `table_name` annotation if we want to.

I also much prefer the workaround for
rust-lang/rust#47311 in this PR to the one I
did in #1529. I'll need to update that PR if this is merged first.
@sgrif sgrif merged commit 0689b0a into master Feb 3, 2018
@sgrif sgrif deleted the sg-rewrite-queryable branch February 3, 2018 21:37
sgrif added a commit that referenced this pull request Feb 3, 2018
Since `QueryableByName` is one of the more recently written derives, it
should have been a really straightforward port. Unfortunately, the
tests for this derive hit multiple rustc bugs

- rust-lang/rust#47983
- rust-lang/rust#47311

I love what we were able to do with the error message here. We could
even go so far as to have the `help` lines point at the struct itself
for the `table_name` annotation if we want to.

I also much prefer the workaround for
rust-lang/rust#47311 in this PR to the one I
did in #1529. I'll need to update that PR if this is merged first.
sgrif added a commit that referenced this pull request Feb 3, 2018
Since `QueryableByName` is one of the more recently written derives, it
should have been a really straightforward port. Unfortunately, the
tests for this derive hit multiple rustc bugs

- rust-lang/rust#47983
- rust-lang/rust#47311

I love what we were able to do with the error message here. We could
even go so far as to have the `help` lines point at the struct itself
for the `table_name` annotation if we want to.

I also much prefer the workaround for
rust-lang/rust#47311 in this PR to the one I
did in #1529. I'll need to update that PR if this is merged first.
sgrif added a commit that referenced this pull request Feb 4, 2018
Since `QueryableByName` is one of the more recently written derives, it
should have been a really straightforward port. Unfortunately, the
tests for this derive hit multiple rustc bugs

- rust-lang/rust#47983
- rust-lang/rust#47311

I love what we were able to do with the error message here. We could
even go so far as to have the `help` lines point at the struct itself
for the `table_name` annotation if we want to.

I also much prefer the workaround for
rust-lang/rust#47311 in this PR to the one I
did in #1529. I'll need to update that PR if this is merged first.
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.

2 participants