issue_comments: 753567508

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions issue performed_via_github_app
https://github.com/simonw/sqlite-utils/pull/203#issuecomment-753567508 https://api.github.com/repos/simonw/sqlite-utils/issues/203 753567508 MDEyOklzc3VlQ29tbWVudDc1MzU2NzUwOA== 9599 2021-01-03T04:48:17Z 2021-01-03T04:48:17Z OWNER

Sorry for taking so long to review this!

This approach looks great to me - being able to optionally pass a tuple anywhere the API currently expects a column is smart, and it's consistent with how the pk= parameter works elsewhere.

There's just one problem I can see with this: the way it changes the ForeignKey(...) interface to always return a tuple for .column and .other_column, even if that tuple only contains a single item.

This represents a breaking change to the existing API - any code that expects ForeignKey.column to be a single string (which is any code that has been written against that) will break.

As such, I'd have to bump the major version of sqlite-utils to 4.0 in order to ship this.

Ideally I'd like to make this change in a way that doesn't represent an API compatibility break. I need to think a bit harder about how that might be achieved.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
 743384829