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

Adds final state draw.io skeleton architecture diagram #65

Merged
merged 2 commits into from
Jul 8, 2021

Conversation

carjug
Copy link
Contributor

@carjug carjug commented Jul 7, 2021

What does this change?

Adds png for our desired final state architecture diagram to our documentation

What does this change look like?

Diagram

GitHub cards

Completes part of #27

@carjug carjug requested review from alexsoble and amymok July 7, 2021 20:41
@carjug
Copy link
Contributor Author

carjug commented Jul 7, 2021

Question for the group: do we also need to add an ADR for the architecture we've come up with? That feels like it'd be covered by the tech stack ADR mostly, but....I wasn't quite sure.
CC: @amymok @alexsoble

Copy link
Contributor

@amymok amymok left a comment

Choose a reason for hiding this comment

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

Looks good! I would suggest to rename the file to something like final_state_architecture_diagram.png, architecture_diagram.png, etc. because as-is reads like state_diagram, and it may be confusing that this is an actual "state diagram" and not an architecture diagram.

@amymok
Copy link
Contributor

amymok commented Jul 7, 2021

Question for the group: do we also need to add an ADR for the architecture we've come up with? That feels like it'd be covered by the tech stack ADR mostly, but....I wasn't quite sure.
CC: @amymok @alexsoble

I think we need to explain and record some of the choices as ADRs if we haven't yet. For example, CircleCI, S3.

@carjug
Copy link
Contributor Author

carjug commented Jul 7, 2021

Looks good! I would suggest to rename the file to something like final_state_architecture_diagram.png, architecture_diagram.png, etc. because as-is reads like state_diagram, and it may be confusing that this is an actual "state diagram" and not an architecture diagram.

Got it! Fixed

@alexsoble
Copy link
Contributor

Looks good to me as a starting point! A good visual we can use to talk with our PM and PO!

I approve this PR, but I think we'll also want PM and maybe PO review for this one.

@alexsoble
Copy link
Contributor

I can also easily import the PNG into draw.io and edit it, which is great!

Copy link
Contributor

@amymok amymok left a comment

Choose a reason for hiding this comment

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

Looks great!

@carjug
Copy link
Contributor Author

carjug commented Jul 8, 2021

@amymok to respond to your second comment, I think the CircleCI choice is covered by #55. The choice for S3 is not covered anywhere but I'm not sure if we should add it as a part of this issue or create a new one? either is fine for me.

@amymok
Copy link
Contributor

amymok commented Jul 8, 2021

Oh I do not mean adding to this issue, I am just saying we should probably have an ADR on those.

@amymok amymok merged commit 5d4e471 into main Jul 8, 2021
@amymok amymok deleted the cj/27/skeleton-system-architecture branch July 8, 2021 01:49
@ninamak
Copy link
Contributor

ninamak commented Jul 8, 2021

Would it work to walk me through this diagram during our Demo block on Mon (7/12)?

Who is going to create cards for the ADRs that need to be written up?

@carjug
Copy link
Contributor Author

carjug commented Jul 8, 2021

@ninamak Sure, I can walk you through it.
There is one card that needs to be made for one ADR, I can do that.

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.

4 participants