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

Add support for APPENDUID response data #232

Merged
merged 4 commits into from
Aug 11, 2022

Conversation

bitfehler
Copy link
Contributor

@bitfehler bitfehler commented Jul 6, 2022

If the UIDPLUS extension is supported, the server will reply to
APPEND commands with the UID of the new message. This can even be a
list of UIDs if the MULTIAPPEND extension is also supported.

Make this information available to the user as the result of an
AppendCmd. The added doc strings have links to the relevant RFCs.

Related to #131.


This change is Reviewable

@bitfehler
Copy link
Contributor Author

If this is of interest, I'd be happy to add COPYUID as well...

Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you! And yes, COPYUID would be good to add, though happy to take that in a separate PR if you'd prefer.

src/types/appended.rs Outdated Show resolved Hide resolved
src/client.rs Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 14, 2022

Codecov Report

Merging #232 (1732482) into master (f616ea9) will increase coverage by 0.38%.
The diff coverage is 90.90%.

Impacted Files Coverage Δ
src/parse.rs 85.45% <90.00%> (+0.51%) ⬆️
src/client.rs 83.95% <100.00%> (+0.02%) ⬆️
src/types/appended.rs 100.00% <100.00%> (ø)

Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, can you please add a test for the new functionality too?

@bitfehler
Copy link
Contributor Author

Added a parse test much like the other one, I hope that's cool for now. I am happy to look into the integration tests, however I will be AFK for two weeks.

src/parse.rs Show resolved Hide resolved
@jonhoo
Copy link
Owner

jonhoo commented Jul 17, 2022

Looks like there's also now a conflict in src/parse.rs.

If the `UIDPLUS` extension is supported, the server will reply to
`APPEND` commands with the UID of the new message. This can even be a
list of UIDs if the `MULTIAPPEND` extension is also supported.

Make this information available to the user as the result of an
`AppendCmd`. The added doc strings have links to the relevant RFCs.

Related to jonhoo#131.
@bitfehler
Copy link
Contributor Author

Sorry for the wait! I rebased on current master and added another test for the MULTIAPPEND response. I hope this is fine for this PR. I'll start looking into the integration tests to see if I can add some for all the stuff I added now.

Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@jonhoo jonhoo merged commit 8c22502 into jonhoo:master Aug 11, 2022
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