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

Add BridgedEngine to sync15-traits #2970

Merged
merged 4 commits into from Apr 23, 2020
Merged

Add BridgedEngine to sync15-traits #2970

merged 4 commits into from Apr 23, 2020

Conversation

thomcc
Copy link
Contributor

@thomcc thomcc commented Apr 15, 2020

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1626128 / https://jira.mozilla.com/browse/SYNC-1033

Last commit might be all we need to do for https://bugzilla.mozilla.org/show_bug.cgi?id=1628892 / https://jira.mozilla.com/browse/SYNC-1088

I also renamed interrupt to saci-interrupt and made it manually implement errors. Failure is a really slow dependency to compile, and saved us like 5 lines, and also this is a dep very specific to our team and so it kinda should reflect that (I've always kind of hated how broad it's name was). I used saci- over as- since the latter is an english word.

Pull Request checklist

  • Quality: This PR builds and tests run cleanly
    • automation/all_tests.sh runs to completion and produces no failures
    • Note: For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry in CHANGES_UNRELEASED.md or an explanation of why it does not need one
    • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
  • Dependencies: This PR follows our dependency management guidelines
    • Any new dependencies are accompanied by a summary of the due dilligence applied in selecting them.

@thomcc thomcc requested a review from linabutler April 15, 2020 23:04
Copy link
Contributor

@linabutler linabutler left a comment

Choose a reason for hiding this comment

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

I like the modernization of interrupt and the telemetry addition, this looks great!

Not to bikeshed too much 🚲🎨🏠 but how do you feel about interrupt-support as a name, to match sql-support and ffi-support? I don't mind saci-, it's just an acronym we haven't use publicly yet—but, if we start using it elsewhere, this seems totally fine!

Thanks, @thomcc!

@thomcc thomcc added the checkin-needed Auto-merge this PR if there's an approved review and CI passes label Apr 23, 2020
@thomcc thomcc merged commit 3c5cfbb into master Apr 23, 2020
@thomcc thomcc deleted the bridged-engine branch April 23, 2020 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
checkin-needed Auto-merge this PR if there's an approved review and CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants