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

Merged
merged 16 commits into from
Nov 7, 2024
Merged

Conversation

ioanmo226
Copy link
Collaborator

@ioanmo226 ioanmo226 commented Oct 7, 2024

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 ioanmo226 requested a review from sosnovsky as a code owner November 6, 2024 14:01
@ioanmo226
Copy link
Collaborator Author

@sosnovsky Ready for review!
Thank you

Copy link
Collaborator

@sosnovsky sosnovsky left a comment

Choose a reason for hiding this comment

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

Works great 👍

appium/api-mocks/lib/configuration-types.ts Outdated Show resolved Hide resolved
@ioanmo226 ioanmo226 requested a review from sosnovsky November 7, 2024 12:27
Copy link
Collaborator

@sosnovsky sosnovsky left a comment

Choose a reason for hiding this comment

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

👍

@sosnovsky sosnovsky enabled auto-merge (squash) November 7, 2024 12:29
@sosnovsky sosnovsky merged commit 31c7e8d into master Nov 7, 2024
10 checks passed
@sosnovsky sosnovsky deleted the 2566-add-email-signatures-support branch November 7, 2024 13:00
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