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

New GitHub action workflow for pages #1165

Conversation

blackgirlbytes
Copy link
Contributor

@blackgirlbytes blackgirlbytes commented Aug 2, 2022

What kind of changes does this PR include?

  • New or updated content 🎉

Description

  • Closes Add new GitHub Action workflow to Deploy to GitHub Pages #1164
  • What does this PR change? I updated GitHub Action workflow to deploy Astro on GitHub Pages. GitHub officially supports deploying static websites written in any framework on GitHub Pages. There's a specific one that works well for Astro.
  • Did you change something visual?
    BEFORE

Screen Shot 2022-08-02 at 5 08 27 PM

AFTER

image

Working examples:
From engineer at GitHub on the Pages-Actions team: https://github.com/tcbyrd/astro-portfolio-demo/blob/main/.github/workflows/astro-build.yml
My own repo: https://github.com/blackgirlbytes/blackgyalbites-astro has a working example as well

@netlify
Copy link

netlify bot commented Aug 2, 2022

Deploy Preview for astro-docs-2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 28c0d0a
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/62eb72fd02e27e000815b88a
😎 Deploy Preview https://deploy-preview-1165--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@blackgirlbytes blackgirlbytes changed the title 1164 new GitHub action workflow for pages New GitHub action workflow for pages Aug 2, 2022
@yanthomasdev yanthomasdev added the improve documentation Enhance existing documentation (e.g. add an example, improve description) label Aug 2, 2022
Copy link
Contributor

@Pukimaa Pukimaa left a comment

Choose a reason for hiding this comment

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

Almost LGTM!

src/pages/en/guides/deploy/github.md Show resolved Hide resolved
src/pages/en/guides/deploy/github.md Show resolved Hide resolved
Copy link
Contributor

@Pukimaa Pukimaa left a comment

Choose a reason for hiding this comment

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

Another few comments

src/pages/en/guides/deploy/github.md Outdated Show resolved Hide resolved
src/pages/en/guides/deploy/github.md Show resolved Hide resolved
src/pages/en/guides/deploy/github.md Show resolved Hide resolved
@delucis
Copy link
Member

delucis commented Aug 2, 2022

Thanks so much for this @blackgirlbytes and thanks @Pukimaa for coming in with the review! I'm not going to get a review in this evening but I'll take a close look tomorrow morning.

Appreciate the contribution! 🙌

@blackgirlbytes
Copy link
Contributor Author

My pleasure. I'm just excited to contribute to Astro. It's such a cool project! And thanks @pukima for catching those issues

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks so much for this update @blackgirlbytes!

I’ve made a few suggestions. One thing I can’t add inline is that we should remove the :::tip block below the code sample as it’s linking to the github-pages-action which is no longer in use in the updated action.

src/pages/en/guides/deploy/github.md Outdated Show resolved Hide resolved
# Allows you to run this workflow manually from the Actions tab on GitHub.
workflow_dispatch:

# Allow this job to push changes to your repository
Copy link
Member

Choose a reason for hiding this comment

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

Nit: looks like this needs slightly different indentation to align with the block it refers to:

image

Suggested change
# Allow this job to push changes to your repository
# Allow this job to push changes to your repository

src/pages/en/guides/deploy/github.md Outdated Show resolved Hide resolved
src/pages/en/guides/deploy/github.md Show resolved Hide resolved
@sarah11918
Copy link
Member

Hey @blackgirlbytes! Welcome to the Docs repo! 🥳

I'm just making a note here to remind us that #1110 is also open and touching the page, but I think it starts immediately after the changes you're making here, so we should be good! 😅

Would everyone here mind taking a peek at THAT PR, too, to make sure that all these changes make sense together? @delucis @Pukimaa ? I made some changes on behalf of the author, but haven't heard back from them in a while. So, if that all reads fine with what's going on in this PR, then I'm happy for us to merge that one, as a smaller change first, then update this branch here and continue working out this one.

If someone spots something in that PR that does NOT go down smooth with what's going on here, then let's figure that out while we have some active voice in this thread. 😄 Thanks everyone, for this great team effort!

Rizel Scarlett and others added 2 commits August 3, 2022 09:14
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Copy link
Contributor

@tcbyrd tcbyrd left a comment

Choose a reason for hiding this comment

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

Few suggestions to align with our other starter workflows

src/pages/en/guides/deploy/github.md Outdated Show resolved Hide resolved
src/pages/en/guides/deploy/github.md Outdated Show resolved Hide resolved
src/pages/en/guides/deploy/github.md Outdated Show resolved Hide resolved
Rizel Scarlett and others added 3 commits August 3, 2022 09:59
Co-authored-by: Tommy Byrd <tcbyrd@github.com>
Co-authored-by: Tommy Byrd <tcbyrd@github.com>
Co-authored-by: Tommy Byrd <tcbyrd@github.com>
Co-authored-by: James M. Greene <JamesMGreene@github.com>
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks again for this — really glad we could simplify things for users!

And thanks for surviving what was maybe one of the most intense review processes I’ve seen. We’re usually thorough here, but my word — this was another level! 😅

@delucis delucis merged commit d524379 into withastro:main Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improve documentation Enhance existing documentation (e.g. add an example, improve description)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new GitHub Action workflow to Deploy to GitHub Pages
7 participants