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
Add support for incoming emails #22056
Add support for incoming emails #22056
Changes from 3 commits
5713613
47029bf
e159bb8
7c818d5
ecb606d
90860b6
f7e74bb
b82efef
d85d448
f46ca3f
ed3c24f
28a34fe
80c1466
f7ee50e
35aa103
4e38af3
5387897
a334f55
1834a3f
0beb48a
6b2fe10
b0a2c8f
5094c0d
5066d29
9c54fcf
93b3469
661c367
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.
Also, does this accept the
<x>MB
notation, or do you need to set an explicit byte size?That's not clear from the description.
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.
Currently it does not accept a human readable notation. I added that only for the packages. Should I make it available for other setting types too?
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.
Maybe
parsed.Address
is better?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.
Must be identical if the parsing worked without errors?
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.
Shouldn't we do something with
r
?Returning it maybe?
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.
Line 26 uses it to decode the data.
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 know, but I don't quite understand the purpose of this method:
It is called
UnpackData
, so it should unpack the data, right?As it only returns an error, the only option to return modified data is that it modifies the
buf
directly.However, given the reader you declare, this looks unlikely to me…
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.
Have a look at the test code. It's the same interface other
Decode
methods use. It uses the raw data inbuf
and unpacks it to the variables passed indata
.