-
Notifications
You must be signed in to change notification settings - Fork 12
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
[ISSUE 299] Set up front-end infrastructure in AWS #381
Conversation
…template overwrite because we've already made our own
…tf development for ease
b5b7a31
to
2d4598d
Compare
51658df
to
37a1f68
Compare
8f30575
to
89f46d7
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.
Amazing work and progress!
(blocking): We need to address the docs/
folder. Ultimately it would be integrated with the documentation
folder.
For this PR I think we could remove the docs/
folder and create a separate ticket to properly integrate it, or integrate the folder in this PR.
The docs/infra
folder could be moved to documentation/infra
or documentation/architecture
. The ADRs should have a happy home in documentation/decisions/
, but that could be its own PR as well.
If there is energy to integrate the docs
in this PR, would work as well.
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.
suggestion (blocking):
The documentation/decisions/infra
are not formatted in the same way as the others. We have also not discussed whether to separate ADRs into separate folders AFAIK. I would recommend removing the infra ADRs for another ticket or to include the ADRs in the newer date format.
documentation/infra/infra
material could be moved to documentation/infra
and the redundant infra/infra
folder removed.
I would recommend removing /documentation/infra/code-reviews.md
and releases.md
as is infra specific.
Actually don't think it's worthwhile to change the format. We are just creating more busy work for ourselves whenever we pull in changes from the infrastructure template. The format doesn't need to be blocking, does it? Also don't think it's worthwhile blocking this PR based on a discussion on if it's okay to have sub-folders in our ADR directory. I question the logic of having one super-giant directory of dated ADRs with no index and no organization anyway. tl;dr there's nothing preventing us from iterating on this, creating another ticket. No need to delete. Let's get it merged. Thanks for the other comments, makes sense and things that got lost in this giant PR. |
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.
Thanks for updating the docs folder. Looks a lot cleaner now. I think the difference in format introduces a new change that could be best handled in a separate ticket. I would typically hold off on introducing that change until the ticket is created, but don't want to hold up this amazing work. Let's discuss when we get a chance.
Summary
Fixes #299
Time to review: 20 mins
Changes proposed
Integrates https://github.com/navapbc/template-infra into grants-equity
Context for reviewers
Infrastructure has been deployed to AWS and front-end app is running there in dev cluster. Includes account level infrastructure. :)
http://frontend-dev-143463875.us-east-1.elb.amazonaws.com/