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

feat(docs): improve hosting with AWS Amplify document #10906

Merged
merged 17 commits into from Jan 10, 2019
Merged

feat(docs): improve hosting with AWS Amplify document #10906

merged 17 commits into from Jan 10, 2019

Conversation

swaminator
Copy link
Contributor

Description

The Amplify Console (launched at re:invent 2018) provides continuous deployment and hosting for modern web apps (single page apps and static site generators). Continuous deployment allows developers to deploy updates to their web app on every code commit to their Git repository. Hosting includes features such as globally available CDNs, easy custom domain setup + HTTPS, feature branch deployments, and password protection.

This tutorial walks users through deploying a Gatsby starter enabled with authentication to the Amplify Console.

@swaminator swaminator requested a review from a team January 8, 2019 00:50
@swaminator swaminator requested a review from a team as a code owner January 8, 2019 00:50
@LekoArts
Copy link
Contributor

LekoArts commented Jan 8, 2019

I'm not familiar with S3/Amplify but why did you change the current doc and not create a new one? There's currently this PR open to change the doc:
#10694

Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

Technically - this looks good. Excited to have this--been meaning to check out Amplify!

We should probably rename the document though, correct? And then set up a re-direct or re-establish the old "Deploying to S3/Cloudfront" document before it was swapped with the Amplify content.

Is there a reason you wouldn't use Amplify or is there still value in documenting both approaches?

@DSchau DSchau changed the title Hosting with AWS Amplify feat(docs): improve hosting with AWS Amplify document Jan 8, 2019
@swaminator
Copy link
Contributor Author

swaminator commented Jan 8, 2019

@DSchau @LekoArts nice to meet you guys. Here's my plan for next steps - let me know if you have any comments:

  1. I'll update the PR to change the filename to 'deploying-to-aws-amplify'.

  2. Wrt maintaining two docs for Amplify and S3/CloudFront, my recommendation is to setup a redirect from the old URL to the new one rather than maintaining two docs. I just read fix(docs): tweak s3/cloudfront doc to actually use s3 and cloudfront #10694 and it looks like the author created the PR without checking out the Amplify Console (understandably so as we only launched a month ago). The Amplify Console is the fastest way to deploy a Gatsby app on AWS - it covers a lot of usecases that PR fix(docs): tweak s3/cloudfront doc to actually use s3 and cloudfront #10694 doesn't specifically address (continuous deployment, feature branch deploys, redirects, custom domains for apex+www, SSL, reverse proxies, atomic deploys).

Once you share your thoughts on point number 2 above, I'll go ahead and update the PR.

@LekoArts
Copy link
Contributor

LekoArts commented Jan 8, 2019

https://www.gatsbyjs.org/docs/gatsby-style-guide/#share-best-practices-whenever-possible

The best practice and why is it the best (if different than 3)

Does Amplify completely eliminate the need for S3/Cloudfront? I still feel documenting what #10694 writes is worth it.

@DSchau
Copy link
Contributor

DSchau commented Jan 8, 2019

@swaminator I'm totally cool with creating a new document, but I do (still) think it's valuable to keep the documentation for deploying to s3/cloudfront. Perhaps we create a new document here (deploying-to-amplify)?

Primarily, this is because I'd imagine there are (still) reasons for using s3/cloudfront in certain scenarios? We can highlight/show Amplify as the happy path, but I'm sure there are some who'd set up the infra themselves, so I'd like to keep both if possible. Am I off base here? If so--let me know and we can set up the redirect and get that all ship shape!

@swaminator
Copy link
Contributor Author

swaminator commented Jan 8, 2019

@DSchau: Sounds good to me! It's a good idea to document both approaches as well as highlight that the Amplify Console is the fastest way to setup your Gatsby deployment, but if you want more fine-grained control of your resources, S3/CloudFront is a good option.

Questions for you:

  1. I couldn't find where to update the main marketing page at https://www.gatsbyjs.org/ with a link to 'AWS Amplify' in the DEPLOY section. Could you point me to the right file?
  2. Could you please confirm what redirect you will be setting up?

Next steps for me: I'll create a file called 'deploying-to-aws-amplify' and update the PR. Please let me know if there's anything else I need to do.

@swaminator
Copy link
Contributor Author

swaminator commented Jan 8, 2019

Just added the following:

  1. Created new file: deploying-to-aws-amplify
  2. Fixed doc link to point to AWS Amplify doc

Outstanding items:

  1. Update marketing page 'DEPLOY' section to direct users to 'AWS Amplify' (@DSchau ?)
  2. Setup redirect (@DSchau ?)

@jariz
Copy link
Contributor

jariz commented Jan 10, 2019

Hi @swaminator! Just found out about this issue.
Could you elaborate on why you reset the s3-cloudfront file back to the Amplify CLI setup instructions? (d9bfef4)

You're correct in me not being familiar with the Amplify console, and I do agree that it's the 'happy path', but I think in cases where you are still requiring to have control over your own infra (like @DSchau says) gatsby-plugin-s3 would still be the best way to go over Amplify CLI, but convince me otherwise if you do not agree.

@DSchau
Copy link
Contributor

DSchau commented Jan 10, 2019

@jariz that's actually (for some reason) the way the S3 document is currently set up in our repo -> https://github.com/gatsbyjs/gatsby/blob/master/docs/docs/deploying-to-s3-cloudfront.md

We should reset it to the way it was (before the introduction of Amplify) but it seems like you're already working on that?

So I think the best course of action here is to:

  1. Review both PRs and get them merged (probably this one first)
  2. Keep the redirects as is (e.g. don't redirect) because they should be separate

Does anyone disagree? If not--I'll give these both a once over today and get them merged in. Thank you both for your patience!

@DSchau
Copy link
Contributor

DSchau commented Jan 10, 2019

Alright - after a barrage of PRs, I think this is ready--presuming the linter passes.

What I did was:

  • Get the linter passing
  • Update the sidebar deploy instructions (change all Hosting on -> Deploying with)
  • Alphabetize the sidebar deploy steps (e.g. AWS Amplify comes before Github Pages which comes before S3 and Cloudfront)
  • Add back the S3 and Cloudfront deploy guide to the sidebar--I think both are valid

@swaminator
Copy link
Contributor Author

@jariz: I initially merged the work, and then later thought it would be best to separate the two PRs so we can get them approved individually.

@DSchau: thanks for getting the linter passing. This PR looks good to me.

Copy link
Contributor

@shannonbux shannonbux left a comment

Choose a reason for hiding this comment

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

Only request is in comment I left. Other than that, this looks great! Thanks for putting in the work to make this change!

@@ -32,8 +32,10 @@
link: /docs/deploying-to-now/
- title: Deploying to GitLab Pages
link: /docs/deploying-to-gitlab-pages/
- title: Hosting on Netlify
- title: Deploying to Netlify
Copy link
Contributor

Choose a reason for hiding this comment

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

if we change the title here, we would need to change it in the doc, the URL, and also create a redirect. It seems like the doc is about hosting though. Am I wrong about that?

Copy link
Contributor

Choose a reason for hiding this comment

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

@shannonbux I'm not sure there's a meaningful difference--and this seems more consistent (e.g. all deploying guides start with Deploying to-? I can revert this change, though.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh whoops we actually do have two separate docs here.

Deploying to Netlify
Hosting on Netlify

... should we? It seems like the Hosting on Netlify document is a little more fully featured.

Copy link
Contributor

Choose a reason for hiding this comment

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

actually - let's just side step this. I'll revert this change.

@DSchau DSchau dismissed shannonbux’s stale review January 10, 2019 21:50

(reverted to existing structure; thanks Shannon!)

@DSchau
Copy link
Contributor

DSchau commented Jan 10, 2019

Alright - finally merging this! Waiting for the status checks to ✅

@DSchau DSchau merged commit bbeae87 into gatsbyjs:master Jan 10, 2019
@gatsbot
Copy link

gatsbot bot commented Jan 10, 2019

Holy buckets, @swaminator — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. (Currently we’ve got a couple t-shirts available, plus some socks that are really razzing our berries right now.)
  2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

@DSchau
Copy link
Contributor

DSchau commented Jan 10, 2019

@swaminator merged! Thanks so much for the PR. Really looking forward to checking out Amplify--sometime soon hopefully!

@swaminator
Copy link
Contributor Author

Thanks @DSchau! Looks good and great working with you.
One request: Is there anyway to get a link in the DEPLOY section on the homepage at https://www.gatsbyjs.org/?

gpetrioli pushed a commit to gpetrioli/gatsby that referenced this pull request Jan 22, 2019
* Added AWS Amplify Console

* Add AWS Amplify images and gifs

* Updated doc to deploying with AWS Amplify

* Update AWS Amplify doc link

* Update deploying-to-s3-cloudfront.md

* Update nav

* Added S3 docs from PR: gatsbyjs#10694 and created doc for AWS Amplify deployments

* updated docs based on PR feedback

* added amplify doc

* Reset s3/Cloudfront docs to the current docs

* Update deploying-to-aws-amplify.md

* Update deploying-to-s3-cloudfront.md

* Update deploying-to-aws-amplify.md

* Update doc-links.yaml

* Update doc-links.yaml

* Update deploying-to-aws-amplify.md

* Update doc-links.yaml
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

5 participants