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

improve JMAP compatibilities with stalwart #909

Merged
merged 5 commits into from
Mar 5, 2024

Conversation

Shadow243
Copy link
Member

This Pull Request follows a proposal that had been made: #648 adding support for JMAP in Cypht.

@Shadow243 Shadow243 marked this pull request as draft February 19, 2024 11:27
@Shadow243
Copy link
Member Author

Currently, we have left it as draft pending this fix #902 which will fix email sending as it is not working for now. We have been not able to test send mail

@landryb
Copy link
Contributor

landryb commented Feb 23, 2024

fwiw it seems 4bc4cea reverts the previous commits in hm-jmap

@josaphatim josaphatim requested a review from kroky March 1, 2024 04:50
@Shadow243
Copy link
Member Author

fwiw it seems 4bc4cea reverts the previous commits in hm-jmap

We started with your commits then added another, we now have your 4 commits plus 1(5 commits)

@landryb
Copy link
Contributor

landryb commented Mar 4, 2024

fwiw it seems 4bc4cea reverts the previous commits in hm-jmap

We started with your commits then added another, we now have your 4 commits plus 1(5 commits)

yes, but unless i'm mistaken and dont understand the github UI anymore, if you look at the actual PR contents on https://github.com/cypht-org/cypht/pull/909/files there's only a oneliner change which comes from 4bc4cea, and all the other changes from the previous 4 commits are reverted in it... so the complete PR doesn't actually contain 'my commits'

@kroky
Copy link
Member

kroky commented Mar 4, 2024

@landryb is right - the original commits were reverted as part of another one - namely, this one: 4bc4cea

@Shadow243 can you please revert that and start with the original 4 commits and then introduce the changes you want? I think we should address the commenting of the isPrimary check - if that's not needed, just remove the code.

@Shadow243 Shadow243 force-pushed the stalwart-jmap-648 branch from 4bc4cea to 441258b Compare March 5, 2024 02:43
@Shadow243 Shadow243 force-pushed the stalwart-jmap-648 branch from 441258b to e9cecf9 Compare March 5, 2024 02:47
@Shadow243
Copy link
Member Author

Shadow243 commented Mar 5, 2024

Copy that. Changes separated here: e9cecf9

@kroky
Copy link
Member

kroky commented Mar 5, 2024

Thanks @Shadow243!

@landryb do you plan to push more commits here or we can merge now?

@landryb
Copy link
Contributor

landryb commented Mar 5, 2024

i havent been able to test more jmap since 1.5 year, so dont wait for me :) hopefully someday i'll be able to retest cypht against stalwart-jmap...

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.

3 participants