search

symmetryinvestments / imap-d

D library for IMAP (JMAP is a work-in-progress but the basics work)
14 stars 10 forks source link

Added SIL example script and fix a couple of bugs. #37

Closed otrho closed 3 years ago

otrho commented 3 years ago

This branch is 8 commit ahead so I thought perhaps it's about time to merge it.

Some of it might be controversial, especially changes to request.d and especially the 'feature' for FETCH requests which allow sequence IDs by tagging them with a '#'. So each commit should probably be inspected on its own, especially since a couple of them are uncrustify passes.

Laeeth commented 3 years ago

Reverts #37

I reverted this because I think the reverted MR changes behaviour in a way that isn't helpful. The reason to stick UID before the FETCH etc is that way it uses the UID that won't change. Without the UID then it uses a message number that can change when you aren't expecting it - legacy behaviour maintained for reasons of backward compatibility.

In the event that this was what we wanted, a significant change like this should be a MR of it's own explaining the reason for and consequence of the changes. But if my memory serves me correctly this is not what we would want.

So please could you stick the UIDs back and resubmit the PR.

Thanks.

otrho commented 3 years ago

The change was actually backward compatible -- it still supported UIDs. I'll create a new issue to outline what I did and perhaps to discuss whether it's what we want.

On Wed, 14 Oct 2020, at 12:17 AM, Laeeth Isharc wrote:

Reverts #37 https://github.com/symmetryinvestments/imap-d/pull/37

I reverted this because I think the reverted MR changes behaviour in a way that isn't helpful. The reason to stick UID before the FETCH etc is that way it uses the UID that won't change. Without the UID then it uses a message number that can change when you aren't expecting it - legacy behaviour maintained for reasons of backward compatibility.

In the event that this was what we wanted, a significant change like this should be a MR of it's own explaining the reason for and consequence of the changes. But if my memory serves me correctly this is not what we would want.

So please could you stick the UIDs back and resubmit the PR.

Thanks.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/symmetryinvestments/imap-d/pull/37#issuecomment-707730894, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABVSYE4BZ4TU7ZUPTGZFQLSKRHNHANCNFSM4SMO55VQ.

Laeeth commented 3 years ago

I guess it should be easy to do the right thing and hard to do the wrong thing. I didn't have time to look too deeply though. But John persuaded me of the benefits of breaking things into smaller PRs that stand on their own and maybe we could do this here.