Skip to content

Conversation

@Expurple
Copy link
Member

There was some annoying repetition in this area. TableName and ColumnName each had three copy-pasted From impls. Now they have one, which will be easier to copy-paste when I add TypeName and FunctionName. I came up with this while re-doing #922 to add TypeName.

Related to #927.

types.rs is getting too long and starting to have visible disjoint "submodules", so I started splitting it.

PR Info

New Features

  • The new traits are technically an addition, but I wouldn't advertise it. They are not meant to be used directly. I've documented this on the traits.

@Expurple Expurple requested review from Huliiiiii and tyt2y3 August 12, 2025 14:46
@Huliiiiii
Copy link
Member

How about renaming into_2_parts to into_pair and into_3_parts to into_triplet?

@Expurple Expurple marked this pull request as draft August 12, 2025 15:03
@Expurple
Copy link
Member Author

Wait a second. Now I'm not sure if this will actually help me to model TypeName conversions. I'll post an update under #922, let's discuss there. Changed to draft for now

@Expurple
Copy link
Member Author

How about renaming into_2_parts to into_pair and into_3_parts to into_triplet?

I don't mind, both seem fine to me

@Expurple Expurple marked this pull request as ready for review August 13, 2025 16:55
@Expurple Expurple merged commit f827b75 into SeaQL:master Aug 13, 2025
29 checks passed
Expurple added a commit that referenced this pull request Aug 13, 2025
As discussed here:
#827 (comment)

## PR Info

- Closes #827

- Dependencies:
  - #921
  - #953

## Changes

The PR has been updated. The up-to-date description is in the changelog
in the diff.

---

**Original, outdated note:**

Merge after #921. Marking as draft until then. The first commit comes as
a dependency from that PR. If you want to review the diff before merging
that PR, review only the second commit.
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