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

[WIP] NOTIFY Command #64

Closed
wants to merge 15 commits into from
Closed

Conversation

jkaessens
Copy link

@jkaessens jkaessens commented Feb 6, 2018

Once finished, this implements the IMAP NOTIFY extensions as discussed in issue #63. Currently work in progress.

Todo:

  • handle untagged STATUS responses in mailbox parser (done in 5d19d4f)
  • handle NOTIFY SET STATUS where STATUSes are sent right after the NOTIFY command (done in f6b6c96)
  • handle asynchronous STATUSes (done in 6e48c77)
  • implement @jonhoo's suggestions about better style (done in 855abc5)
  • make all results ZeroCopyResult

@coveralls
Copy link

coveralls commented Feb 6, 2018

Coverage Status

Coverage decreased (-4.8%) to 84.726% when pulling 855abc5 on jkaessens:notify into dc21eae on mattnenterprise:master.

@jkaessens
Copy link
Author

@jonhoo Hey, it would be nice if you could take another look at the code. I implemented all that has been missing (for now) and also your suggestions. Test runs fine and an actual test against an actual IMAP server with quite a few mailboxes also works.

@jonhoo
Copy link
Collaborator

jonhoo commented May 6, 2018

Hey, sorry, I completely dropped the ball on this one! Can you match up with master (which now also has the structured responses stuff merged) and I'll review? I noticed you'd also like to add ZeroCopyResult, but are there any other changes you expect to make to this?

@jkaessens
Copy link
Author

Hey, no problem. I've been busy with offline stuff anyway. I just matched up with master but I haven't looked into the structured responses and ZeroCopyResult, yet. Maybe I find the time in the next few days.

@jonhoo
Copy link
Collaborator

jonhoo commented May 8, 2018

It'd be nice if you could rebase this onto master instead -- makes the commits a little nicer to follow. I don't think it should be too painful given that master hasn't changed substantially :)

@jonhoo jonhoo added the enhancement New feature or request label Nov 22, 2018
@jonhoo jonhoo added this to the imap 1.0 milestone Nov 22, 2018
@jonhoo
Copy link
Collaborator

jonhoo commented Nov 23, 2018

@jkaessens I've had to move the repository under my own control so that I can set up CI and such correctly following the resolution to #69. Could you please open this PR up again against https://github.com/jonhoo/rust-imap instead? Including a link back to this for reference would probably also be a good idea!

@jonhoo jonhoo closed this Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants