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

Support for vnd.dovecot.pgp-encrypt #658

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

Conversation

kaivol
Copy link

@kaivol kaivol commented Nov 10, 2021

Added (rudimentary) support for the vnd.dovecot.pgp-encrypt sieve filter.

Copy link
Owner

@thsmi thsmi left a comment

Choose a reason for hiding this comment

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

Thank you very much for contributing.

Did a rough check, everything looks good but have not tested it yet. I'll try to do that in the next days. So that this can land soon on the master. I'll also try to do a quick release afterwards.

While reviewing I stumbled upon the following "non technical" glitches:

The original author in the license header is pointing to me. But you did the work so you should add you as the initial author to all the files you created newly. If the file exists then you need to add yourself as an additional author.

Translations are auto generated as soon as it lands in the master. All you need to do is to add the new strings only to the en-us json file. Otherwise crowding gets confused and runs into conflicts which need to be manually resolved. I'll add a readme note to the folder containing the translations.

@kaivol
Copy link
Author

kaivol commented Nov 14, 2021

Thank you for your response!

Please have a look at the last changes as I am not really familiar with this project and its best practices.
Unfortunately this also applies to the vnd.dovecot.pgp-encrypt sieve extension, and I could not find much info about it. Primarily I made sure that my implementation is compatible with the with the way mailbox.org uses the extension.

I also updated the author in the license headers and removed the i18n strings in other languages than en-us.
Originally, I added those because the application would complain about missing translations after adding the English translations only. Did I use the build system wrong or if not, what is the recommended workflow when introducing new translations?

@yoshimo
Copy link

yoshimo commented Nov 14, 2021

Where would i find the dovecot side of this PR and its documentation?

@kaivol
Copy link
Author

kaivol commented Nov 16, 2021

Unfortunately I could not find documentation or source code for the extension.
I asked my provider (mailbox.org) about it, but have not yet received an answer.

@kaivol
Copy link
Author

kaivol commented Nov 23, 2021

According to mailbox.org, no public documentation for this plugin is available.
It was conceived by them and developed by Dovecot (initially exclusively for mailbox.org).

@stale
Copy link

stale bot commented Dec 7, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 7, 2021
@stale stale bot closed this Dec 14, 2021
@thsmi thsmi reopened this Dec 14, 2021
@stale stale bot removed the stale label Dec 14, 2021
@thsmi thsmi added the wip label Dec 14, 2021
@thsmi
Copy link
Owner

thsmi commented Dec 14, 2021

Sorry completely forgot about this pull request.
Will look at it in the next days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants