-
Notifications
You must be signed in to change notification settings - Fork 88
RFC 5256 - SORT command #180
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #180 +/- ##
==========================================
+ Coverage 76.35% 77.75% +1.40%
==========================================
Files 14 15 +1
Lines 1607 1722 +115
==========================================
+ Hits 1227 1339 +112
- Misses 380 383 +3
Continue to review full report at Codecov.
|
I really like the new direction of the parsing, thank you! |
Sorry, I was quite busy these days. I try to update my PR this week! |
I updated my proposal. I opted for a single enum I discovered sth while implementing this feature. The IMAP protocol accepts nested
I am still new at Rust, I may have implemented sth wrongly. Please correct me if needed 😉, I'm open to discussion! |
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 like the way you're going with this 👍 And yes, I think &
is fine for SortCriterion
.
I've left a bunch of mostly minor comments in there that should hopefully not be too onerous to fix!
We're getting really close! |
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.
This is in great shape now, thanks for sticking with it! One minor change which I think I can commit myself through the GitHub interface, and then I'll land it!
Wohoo, it's in! 🎉 |
#178
This change is