Skip to content
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

https://github.com/bbc/digital-paper-edit-api/issues/1 adding diagram… #4

Merged
merged 10 commits into from
Jun 25, 2019

Conversation

emettely
Copy link
Contributor

@emettely emettely commented Jun 21, 2019

…s and updated ADR

Is your Pull Request request related to another issue in this repository ?
bbc/digital-paper-edit-api#1

Describe what the PR does
Updates and fills in the details of the communication between microservices.

State whether the PR is ready for review or whether it needs extra work
Ready

Additional context
We've discussed different cloud approaches, including Microsoft and AWS's best practice guidelines.

Acceptance Criteria

Definitions of Done

  • Runs locally
  • Runs remotely
  • Test passes
  • Demonstrated
  • Deployed to Cosmos on Test and Live
  • Documentation
    • Developer Documentation - [repo's README|]
    • Stakeholder Documentation - [Confluence|]
    • Operational Documentation - [Runbook|]

@emettely emettely marked this pull request as ready for review June 24, 2019 16:26
Copy link
Contributor

@pietrop pietrop left a comment

Choose a reason for hiding this comment

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

We are getting there

  • I might have missed which of the 3 options we are going for, or whether we are still deciding?
  • it's good to have an abstraction in front of PSTT via the STT Proxy service to avoid logic of PSTT to overflow into the rest of our system, and keep a healthy separation of concerns.

docs/adr/2019-04-23-transcript-architecture.md Outdated Show resolved Hide resolved
docs/adr/2019-04-23-transcript-architecture.md Outdated Show resolved Hide resolved
docs/adr/2019-04-23-transcript-architecture.md Outdated Show resolved Hide resolved
@pietrop
Copy link
Contributor

pietrop commented Jun 25, 2019

Images for option 2 and 3 might not show up?
Could you double check the file path etc..

Screen Shot 2019-06-25 at 11 30 37

@emettely emettely requested a review from pietrop June 25, 2019 13:40
Copy link
Contributor

@pietrop pietrop left a comment

Choose a reason for hiding this comment

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

Can we explicitly mention we are going for option 3?

Other then that good to go

(also double check that images show up)

@emettely emettely dismissed pietrop’s stale review June 25, 2019 13:50

I've merged the changes requested :)

@emettely emettely merged commit b04120e into master Jun 25, 2019
@emettely emettely deleted the arch_adr branch June 25, 2019 13:50
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.

None yet

2 participants