Skip to content

Conversation

@Kobzol
Copy link
Member

@Kobzol Kobzol commented Feb 20, 2025

I split these changes from a larger PR that I'm working on that adds the option to set max review PR capacity.

It's a bit fishy that we user the GitHub User type, which is returned from webhooks, to also represent the user type in the DB, but so far they are pretty much the same, so unless we need to separate them, I'd keep it that way to keep the code simpler.

@Kobzol Kobzol requested a review from apiraino February 20, 2025 08:29
Comment on lines +32 to +38
Ok(row.map(|row| {
let username: &str = row.get(0);
User {
id: user_id,
login: username.to_string(),
}
}))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if you SELECT * then you can just Ok(row.into()) but it's a matter of preference ...

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't currently implement From<tokio_postgres::Row> for User anywhere :( Which goes back to my comment that we kind of misuse the GitHub user to also mean the database user.

Introducing a separate type would mean doing some conversions, but I guess that if we want to keep the users table, it's the Right Thing™ to do, in the long term. What do you think? :)

@Kobzol Kobzol merged commit 0fe372b into rust-lang:master Feb 20, 2025
3 checks passed
@Kobzol
Copy link
Member Author

Kobzol commented Feb 20, 2025

I merged it for now, we can do the refactoring of User to DbUser later.

@Kobzol Kobzol deleted the user-db branch February 20, 2025 12:52
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