-
Notifications
You must be signed in to change notification settings - Fork 0
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 flow collections to the API #35
Conversation
92c4fcf
to
71809a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall - I've inlined a few nits and a question about creating new Sources
a1c3601
to
92cc47e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of my comments inline
92cc47e
to
a0e9acd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. As discussed, there are some complications around grouping conflicts and what to do if Flows get deleted (Source inference conflicts, missing slots, etc.), but that's something we can figure that out as we implement these changes.
I was surprised the the JSON schema validator was ok with the codec
and container
properties being on flow-core and only required
in flow-video etc., but it turns out that is valid according to a JSON schema editor.
Add implementation of flow and source collections Update readme.md
b5e68a3
to
4704e1a
Compare
Details
ADR and associated implementation proposing addition of Flow collections to the API, to support multi-essence Flows
Pivotal Story (if relevant)
Story URL: https://www.pivotaltracker.com/story/show/187060669
Related PRs
Where appropriate. Indicate order to be merged.
Submitter PR Checks
(tick as appropriate)
Reviewer PR Checks
(tick as appropriate)
Info on PRs
The checks above are guidelines. They don't all have to be ticked, but they should all have been considered.