Skip to content

refactor(types): borrow structure from imap-types#21

Closed
soywod wants to merge 1 commit intoduesee:mainfrom
soywod:types
Closed

refactor(types): borrow structure from imap-types#21
soywod wants to merge 1 commit intoduesee:mainfrom
soywod:types

Conversation

@soywod
Copy link

@soywod soywod commented Mar 5, 2026

This PR explodes the actual lib.rs into submodules, similar to imap-types.

@soywod soywod mentioned this pull request Mar 5, 2026
3 tasks
@duesee
Copy link
Owner

duesee commented Mar 6, 2026

I very much like the idea but have some concerns. I tried to find/use a tool that would allow me to verify that the code was mostly only moved, such as https://github.com/GumTreeDiff/gumtree. There was some tool able to do it, but I could not find it again. And since I can't realistically review this PR, I have to ask... how did you separate the code into files?

Also... for a refactor... +3358 and -981 lines looks... well... off?

@soywod
Copy link
Author

soywod commented Mar 7, 2026

See #22 (comment) for more context. The code has been separated pretty much the same as imap-types, I will comment each files. The diff is less scary as it looks. While it has been generated, I checked files one by one and it seems legit (at least as a good base to iterate on).

@duesee
Copy link
Owner

duesee commented Mar 7, 2026

I think I should better add another comment before we waste time. I don't feel well taking AI-generated code currently. It feels like too much too quickly, and since I can only do small reviews, I would not feel well to provide this code for other people to use. And yet... I would love to push smtp-codec over the finish line. Also needed it a few times myself.

Good thing is: SMTP is much simpler than IMAP. I think we can get pretty far quickly w/o relying so much on AI.

Shall we talk this through, maybe next week? :-)

@soywod
Copy link
Author

soywod commented Mar 8, 2026

Sure, let's do this!

@soywod soywod closed this Mar 8, 2026
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