Skip to content

Bug 2073610 - [AS] Update SyncParams to allow omitting engine enabl… - #7611

Open
bytesized wants to merge 1 commit into
mozilla:mainfrom
bytesized:5447
Open

bytesized wants to merge 1 commit into
mozilla:mainfrom
bytesized:5447

Conversation

@bytesized

Copy link
Copy Markdown
Contributor

Pull Request checklist

  • Breaking changes: This PR follows our breaking change policy
  • This PR follows the breaking change policy:
    • This PR has no breaking API changes, or
  • Quality: This PR builds and tests run cleanly
    • Tests run successfully
  • Tests: This PR includes thorough tests or an explanation of why it does not
    • Manual testing was performed on Android and iOS, but this does not add automated tests. We expect to add integration tests in SYNC-5417.
  • Changelog: This PR includes a changelog entry in CHANGELOG.md or an explanation of why it does not need one
    • This doesn't make changes that seem significant enough to put in the changelog on their own.
  • Dependencies: This PR follows our dependency management guidelines
    • No dependency changes.

@mhammond

Copy link
Copy Markdown
Member

I don't really understand this - is there ever a time when we'd want to treat None and an empty map as different? What does this enable?

@lougeniaC64

Copy link
Copy Markdown
Contributor

I don't really understand this - is there ever a time when we'd want to treat None and an empty map as different? What does this enable?

@mhammond Are you saying that setting enabled_changes to optional is not necessary for per-device syncing, or that there are implementation details you'd like changed? I thought passing None instead of an empty map might be useful for distinguishing between a legacy sync with no enabled changes and a per-device sync. I created this ticket based on a discussion we had in early August but if there isn't an issue with PDS passing an empty map for enabled_changes this work isn't necessary. cc @bytesized

This branch has not been deployed

No deployments
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