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

Remove action args #22

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Remove action args #22

wants to merge 2 commits into from

Conversation

artemrizhov
Copy link
Owner

No description provided.

@jonashaag
Copy link

👍

Explicit is better than implicit.

There should be one-- and preferably only one --obvious way to do it.

@artemrizhov
Copy link
Owner Author

Thank you for reviewing the changes. One thing is still bothering me. This is the clean arg on the send() method. I feel like it is natural enough:

message.send(clean=True)

But if I remove this then it will require more commands to do simple thing:

message.render()
message.clean()
message.send()

Seems ugly :)

Maybe chaining may help

message.render().clean().send()

But this also seems crazy for me.

@jonashaag
Copy link

Idea 1: Use __ prefixes for internal variables, not sure if this fixes your use case though
Idea 2: In send, construct a new EmailMessage object and send this (this equivalent to always cleaning the object before sending but doesn't change the object's state)

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