Draft SDK type changes for incremental sync #2914
Conversation
| @@ -88,6 +88,9 @@ function hasComment(data: ReflectionData): boolean { | |||
| } | |||
|
|
|||
| function messageForEntity(data: ReflectionData): string { | |||
| if (!data.kindString) { | |||
| throw new Error(`No kindString for ${data.name}, did you forget to add /** ... */ comments?`); | |||
There was a problem hiding this comment.
Drive by improvement:
Before:
After:
patrick-codaio
requested review from
johnli-codaio,
jonathan-codaio,
ekoleda-codaio,
dweitzman-codaio,
ebo-codaio and
a team
|
|
||
| // TODO(patrick): Unhide this | ||
| /** @hidden */ | ||
| prevSyncContinuation?: Continuation; |
There was a problem hiding this comment.
The name prevSyncContinuation doesn't feel that distinct from continuation
I wonder if it's worth doing a quick naming brainstorm & voting exercise in a doc? Personally I might lean toward a name more like incrementalSyncBase to clearly distinguish it from non-incremental sync.
There was a problem hiding this comment.
Ah you're right, I'll make a doc.
api.ts
Outdated
| * TODO(patrick): Unhide this | ||
| * @hidden | ||
| */ | ||
| syncExecutionCompletionMetadata?: SyncExecutionCompletionMetadata; |
There was a problem hiding this comment.
Nitpick: The key name seems overly verbose. Would metadata suffice? Or perhaps completion?
There was a problem hiding this comment.
I like how completion matches continuation, I'll use that.
patrick-codaio
force-pushed
the
patrick-next-sync-continuation
branch
from
62ef2e4
to
814dbb0
Compare
|
This pull request has been inactive for 7 days: labeled as stale. To avoid auto-closure in 7 days, please do one of the following: Add |
patrick-codaio
added
keep-active
No description provided.