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

Rename sqlx(rename) attribute to sqlx(type_name) #940

Merged
merged 4 commits into from
Jan 12, 2021

Conversation

jplatte
Copy link
Contributor

@jplatte jplatte commented Dec 29, 2020

As suggested on Discord.

@jplatte jplatte requested a review from abonander January 4, 2021 14:55
Copy link
Collaborator

@abonander abonander left a comment

Choose a reason for hiding this comment

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

I'm not a big fan of how the deprecation error turned out, at the very least it needs to be clear it's talking about attributes. Also, would you mind adjusting the documentation here

/// * `#[sqlx(rename = "<SQL type name>")]` on struct definition: instead of inferring the SQL type name from the inner

10 | compile_error!("trybuild test needs to fail for stderr checking");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: use of deprecated function `sqlx::_rename`: sqlx(rename) is now called sqlx(type_name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Eh, this isn't great. In our experience, people get confused easily if the error messages aren't direct enough. I would make sure to be clear we're talking about #[sqlx(rename = "...")] and #[sqlx(type_name = "...")] if we can, otherwise it'd just be better to emit an error.

It's nice that it's pointing at the correct span at least.

@jplatte
Copy link
Contributor Author

jplatte commented Jan 4, 2021

Updated the docs and deprecation note, is this better?

@mehcode mehcode merged commit fd0101a into launchbadge:master Jan 12, 2021
@jplatte jplatte deleted the rename-type-name branch January 12, 2021 11:44
@jplatte jplatte mentioned this pull request Feb 4, 2021
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.

3 participants