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

#2566 Add email signature support #2634

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

ioanmo226
Copy link
Collaborator

This PR added email signature support

close #2566 // if this PR closes an issue


Tests (delete all except exactly one):

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@sosnovsky
Copy link
Collaborator

Hi @ioanmo226, let's temporarily disable UI tests, because of their instability and quite random results.
I can check new functionality and added tests locally. After we'll fix ui tests to run consistently (or semaphore upgrades their arm64 machines) - they can be re-enabled.

@ioanmo226
Copy link
Collaborator Author

ioanmo226 commented Nov 6, 2024

I agree but we might miss some issues if we disable UI tests.
For example, I tested deeply about current add email signature feature but missed issue where empty signature string is appended & inconsistent line breaks which was caught by UI tests. (fixed by 03945ce)

@sosnovsky
Copy link
Collaborator

For sure, we'll run all tests before new release, but while working on the remaining features it'll be ok to skip tests and work on failing tests (if there will be any) after fixing CI. As these failing attempts take quite long time, and in November we've already spent almost half of our previous month iOS CI budget.

@ioanmo226
Copy link
Collaborator Author

Ok, I agree

@ioanmo226 ioanmo226 marked this pull request as ready for review November 6, 2024 14:01
@ioanmo226
Copy link
Collaborator Author

@sosnovsky Ready for review!
Thank you

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.

Add email signatures support
2 participants