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

feature: compose: allow not storing the sent emails #37

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

laarmen
Copy link
Contributor

@laarmen laarmen commented Oct 24, 2023

I'm using lieer/gmi to send email, and it has the builtin assumption
that no new messages will be added to the maildir.

See gauteh/lieer#54

@akissinger
Copy link
Owner

I understand making it optional for Dodo to save the sent message, but it doesn't seem particularly helpful to split this small block of code into its own function with an early return, rather than just having a nested if statement.

@laarmen
Copy link
Contributor Author

laarmen commented Oct 24, 2023

Force of habit. Splitting standalone code in free functions with defined semantics helps in testability (not applicable here though), makes it easier to use local reasoning (no variable shadowing, no mutating self), reduces cyclomatic complexity, reduces overall indentation level, etc... which has always helped me in the long run. Ironically, code reuse (which is the initial raison d'être of functions, after all) is really far down the list.

I can redo the patch to do it all inline if you prefer, though.

@akissinger
Copy link
Owner

I can understand that. I think it this case it's overkill though. I'd just shift the 2 statements to after where the setting is read and wrap in an if.

I'm using lieer/gmi to send email, and it has the builtin assumption
that no new messages will be added to the maildir.

See gauteh/lieer#54
@laarmen
Copy link
Contributor Author

laarmen commented Oct 24, 2023

Fair enough, done.

@akissinger akissinger merged commit d2a89d1 into akissinger:master Oct 24, 2023
@laarmen laarmen deleted the no-store-sent branch June 3, 2024 13:06
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