Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: outgoing messages to any address #4512
feat: outgoing messages to any address #4512
Changes from 43 commits
80e2100
c01f90a
5ec38c0
a60bc45
00ca423
ed7418a
b6857dd
d8ce03d
c459552
8e6cbc1
dc59f91
b2150ec
734e98f
837cac0
6179bf8
474272c
0eaca06
ddb0edc
790bbb8
8141672
3ac3372
d547932
06a08c4
dcb1765
dda9aeb
34ddd0b
14a5fd4
6dfacce
47cbee2
ed83bc5
9bd2b13
4e1069a
d28842c
97946b3
3ae852c
e9ccf70
1884d11
2328e68
b71deb0
db2122e
f2f4f11
ec23cb8
32344d0
24ee4df
22c54ce
e2aa37e
461f6d4
6d5c87b
d2ca624
f54ef2d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
message_portal as a name seems weird. As is, it is a noun, but maybe we want it to be more conveying of an action ?
Like, send_message_to_portal ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idk, I am not a native speaker but saying for example "message Esau" sounds totally fine to me and send_message_to_portal is just too long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the difference is that "Message Esau", or "Message Jan" cannot be misconstrued as a singular object, as the separation between action and verb are more pronounced and these are not often used together, but "message portal" can be misunderstood. Especially when it was message_recipient (I know it is has been nuked now but just to make my point clear) i.e. if I call x.message_recipient(), I'll have an initial wonder if I'm grabbing a prop on the object named message_recipient, even if it doesn't make too much sense in the context.
But definitely if we're trying to optimize for terseness then this is the way to go for sure. And this is super nit anyways, I mainly wanted to just give more background on my precious comment 🙃 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok, got it. You have a point. Maybe renaming it as send_message would be the best. It's short and it really can't mean anything else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
{
don't need to be hereThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in f54ef2d