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.

Sign up for GitHub

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

Jump to bottom

mboxlist.c: add 'R' field (rootid, a.k.a. inboxid) to mbentry_t #4838

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

mboxlist.c: add 'R' field (rootid, a.k.a. inboxid) to mbentry_t #4838

wants to merge 1 commit into from

Conversation

ksmurchison
Copy link
Contributor

No description provided.

@ksmurchison ksmurchison requested a review from brong March 11, 2024 01:14
@ksmurchison ksmurchison force-pushed the mboxlist_inboxid branch 3 times, most recently from c2240b9 to b4b5e04 Compare March 12, 2024 03:32
@ksmurchison ksmurchison requested a review from rsto March 12, 2024 04:58
brong
brong approved these changes Mar 12, 2024
View reviewed changes
Copy link
Member

@brong brong left a comment

Choose a reason for hiding this comment

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

Looks good - my only comment is about the syslog during upgrading being a bit meaningless without the mboxname in it.

imap/ctl_cyrusdb.c Outdated Show resolved Hide resolved
@ksmurchison ksmurchison force-pushed the mboxlist_inboxid branch 3 times, most recently from ea83841 to 19c4d9c Compare March 13, 2024 02:03
@ksmurchison ksmurchison requested a review from brong March 13, 2024 02:43
@ksmurchison
Copy link
Contributor Author

@brong I added 'inboxid' code to mailbox_create() which fixes the CaldavAlarm replication tests failing on the v5 branch if you want to re-review.
I also added code to ctl_mboxlist.c to dump the 'U' field

@ksmurchison
Copy link
Contributor Author

@brong Does this look ready? If so, I'll flag it as include-in-fastmail

@brong
Copy link
Member

brong commented Jun 19, 2024

@ksmurchison yes, this looks great - just had a full read through

@rsto rsto removed their request for review August 20, 2024 06:16
@ksmurchison ksmurchison mentioned this pull request Aug 23, 2024
Closed
@ksmurchison
Copy link
Contributor Author

@brong This PR was more complete than my other one so I closed it. However, the old one used an 'R' tag for 'root_mailboxid'. And this one uses 'U' for 'user_inboxid'. Do you have a preference? I'd like to just use 'inboxid' but 'I' is already used for 'uniqueid'. We could use 'B' or 'X'.

@ksmurchison ksmurchison changed the title mboxlist.c: add 'U' field (user inboxid) to mbentry_t mboxlist.c: add 'R' field (rootid, a.k.a. inboxid) to mbentry_t Aug 30, 2024
@rsto rsto self-assigned this Oct 14, 2024
@rsto
Copy link
Member

rsto commented Oct 15, 2024

@ksmurchison Is this still work in progress? Should this get closed or reverted to Draft?

@ksmurchison
Copy link
Contributor Author

I think we still want this. Let's discuss on Monday.

@ksmurchison ksmurchison assigned ksmurchison and unassigned rsto Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brong brong brong approved these changes

Assignees

@ksmurchison ksmurchison

Labels
Do Not Merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ksmurchison @brong @rsto