Skip to content
This repository has been archived by the owner on Jun 5, 2020. It is now read-only.

[SERVICE-542] Refresh FDC3 API docs #120

Merged
merged 21 commits into from Nov 7, 2019
Merged

[SERVICE-542] Refresh FDC3 API docs #120

merged 21 commits into from Nov 7, 2019

Conversation

tim-dinsdale
Copy link
Contributor

Documentation

@tim-dinsdale tim-dinsdale changed the title Dev/tim/service 543 1 [SERVICE-543] Refresh FDC3 API docs Jul 9, 2019
@tim-dinsdale tim-dinsdale changed the title [SERVICE-543] Refresh FDC3 API docs [SERVICE-542] Refresh FDC3 API docs Jul 9, 2019
@tomrobertsOF tomrobertsOF requested a review from a team July 9, 2019 15:46
Copy link
Contributor

@tomrobertsOF tomrobertsOF left a comment

Choose a reason for hiding this comment

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

Some comments to start things off.

src/client/context.ts Outdated Show resolved Hide resolved
src/client/context.ts Outdated Show resolved Hide resolved
src/client/context.ts Show resolved Hide resolved
src/client/context.ts Show resolved Hide resolved
src/client/context.ts Show resolved Hide resolved
src/client/errors.ts Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/provider/intents.ts Outdated Show resolved Hide resolved
src/provider/model/FinEnvironment.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@bryangaleopenfin bryangaleopenfin left a comment

Choose a reason for hiding this comment

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

Lots of comments, but mostly wanting to squish inconsistencies in vocabulary

src/provider/intents.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/errors.ts Outdated Show resolved Hide resolved
src/client/errors.ts Show resolved Hide resolved
src/client/errors.ts Outdated Show resolved Hide resolved
src/client/errors.ts Show resolved Hide resolved
src/client/errors.ts Outdated Show resolved Hide resolved
src/client/context.ts Show resolved Hide resolved
src/client/contextChannels.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/provider/intents.ts Outdated Show resolved Hide resolved
src/provider/intents.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/main.ts Show resolved Hide resolved
src/client/contextChannels.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@tomrobertsOF tomrobertsOF left a comment

Choose a reason for hiding this comment

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

Approved pending conflict fixing

src/client/main.ts Outdated Show resolved Hide resolved
src/client/contextChannels.ts Show resolved Hide resolved
src/client/main.ts Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/directory.ts Outdated Show resolved Hide resolved
src/client/main.ts Show resolved Hide resolved
Tom Roberts and others added 6 commits August 29, 2019 12:59
# Conflicts:
#	package-lock.json
#	src/client/context.ts
#	src/client/directory.ts
#	src/client/main.ts
#	src/demo/apps/ChartsApp.tsx
#	src/demo/apps/NewsApp.tsx
#	src/provider/controller/ResolverHandler.ts
#	src/provider/index.ts
#	src/provider/model/AppWindow.ts
#	src/provider/model/FinEnvironment.ts
#	test/demo/utils/fdc3RemoteExecution.ts
#	test/demo/utils/ofPuppeteer.ts
#	test/mocks.ts
… and adding details around changes since the original docs PR.
@bryangaleopenfin
Copy link
Contributor

bryangaleopenfin commented Nov 5, 2019

@pjbroadbent Taken another look, still outstanding comments

…ions options as per FDC3 specs, replaced redundant Intent type with AppDirIntent.
…543-1

# Conflicts:
#	package-lock.json
#	src/client/context.ts
#	test/demo/raiseIntent/withoutTarget.inttest.ts
#	test/demo/utils/fdc3RemoteExecution.ts
…543-1

# Conflicts:
#	src/provider/model/AppWindow.ts
#	test/demo/raiseIntent/withTarget.inttest.ts
#	test/mocks.ts
#	test/provider/Model.unittest.ts
…ack'. Removed unused default intent definitions from provider. Removed duplicate Intent type definitions, added in earlier commit.
Copy link
Contributor

@bryangaleopenfin bryangaleopenfin left a comment

Choose a reason for hiding this comment

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

Sorry, spotted a bunch more things. Think the only ones I really care about are "it's" v. "its". Can live with the more subjective stuff being ignored

package.json Show resolved Hide resolved
src/provider/intents.ts Show resolved Hide resolved
src/client/context.ts Outdated Show resolved Hide resolved
src/client/context.ts Outdated Show resolved Hide resolved
*
* A context object is open for extension with any custom properties/metadata.
* General-purpose context type, as defined by [FDC3](https://fdc3.finos.org/docs/1.0/context-intro).
* A context object is a well-understood datum that is streamable between FDC3 participants. As a result
Copy link
Contributor

Choose a reason for hiding this comment

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

Is "streamable" the right term here?

Copy link
Contributor

Choose a reason for hiding this comment

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

serializable?..

Copy link
Contributor

Choose a reason for hiding this comment

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

shareable?

Copy link
Contributor

Choose a reason for hiding this comment

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

Meh

src/client/contextChannels.ts Show resolved Hide resolved
src/client/contextChannels.ts Outdated Show resolved Hide resolved
src/client/contextChannels.ts Show resolved Hide resolved
src/client/main.ts Outdated Show resolved Hide resolved
src/client/main.ts Show resolved Hide resolved
@pjbroadbent pjbroadbent merged commit e731c27 into HadoukenIO:develop Nov 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants