Skip to content

/ sync

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

Protobuf.JS 6.45; SyncRecord as JS object #28

Merged
merged 2 commits into from Jan 9, 2017
Merged

Protobuf.JS 6.45; SyncRecord as JS object #28

merged 2 commits into from Jan 9, 2017

Conversation

@ayumi
Copy link
Contributor

ayumi commented Jan 8, 2017 •
edited

Fix #6

  1. Ensures SyncRecord IPC message compatibility.

  2. Upgrades protobuf.js to 6.4.5 and converts api.proto to static JS code.

  • Proto: With classes as static code you can't use reflection e.g. SyncRecord.lookup('Site'). This is minor beause you'd just do SyncRecord.Site.
  • Proto: oneof attributes have issues, notably SyncRecord .objectData is stuck as "bookmark". As a workaround I made a helper: serializer.getSyncRecordObjectData()

Test plan for (1):

  • yarn dist to browser-laptop #feature/syncing
  • Go to browser-laptop app/sync.js#L236; change RECEIVE_SYNC_RECORDS to GET_EXISTING_OBJECTS and add console.log(records).
  • Start browser-laptop and visit any URL to make a historySite.
  • Look at the console log and compare the record in sending record with the GET_EXISTING_OBJECTS record. They should be the same.
@ayumi ayumi requested a review from diracdeltas Jan 8, 2017
ayumi added 2 commits Jan 7, 2017
@ayumi
Upgrade protobuf.js to 6.4.5; use proto static code
76f362c
@ayumi
recordUtil.syncRecordAsJS()
Loading status checks…
4990019
@ayumi ayumi force-pushed the feature/sync-record-js-object branch from ee2a462 to 4990019 Jan 8, 2017
@diracdeltas
Copy link
Member

diracdeltas commented Jan 9, 2017

oops, good catch. i changed RECEIVE_SYNC_RECORDS to GET_EXISTING_OBJECTS in brave/browser-laptop@b9467cc
@diracdeltas diracdeltas merged commit d8532cd into staging Jan 9, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ayumi ayumi deleted the feature/sync-record-js-object branch Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diracdeltas diracdeltas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@ayumi @diracdeltas
You can’t perform that action at this time.