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

86 GITHUB_EMBED #9295

Merged
merged 3 commits into from
Nov 7, 2024
Merged

86 GITHUB_EMBED #9295

merged 3 commits into from
Nov 7, 2024

Conversation

rachelwhitton
Copy link
Member

@rachelwhitton rachelwhitton commented Nov 7, 2024

Summary

Removes gatsby-remark-github npm package, removes related config from gatsby-config.js, removes setup instructions from the README.md file, and replaces instances of usage across the docs with either a crosslink or a manual copy of the code snippets instead of using GITHUB_EMBED:

@rachelwhitton rachelwhitton requested a review from a team as a code owner November 7, 2024 19:07
@rachelwhitton rachelwhitton self-assigned this Nov 7, 2024
@rachelwhitton rachelwhitton added Topic: Security dependencies Pull requests that update a dependency file labels Nov 7, 2024
@rachelwhitton rachelwhitton added the Process: High Priority Issue is critical / needs immediate attention label Nov 7, 2024
Copy link

⚡ Deployed with Pantheon Decoupled

This build was successfully deployed with Pantheon. You can track the build logs here.

👀 Preview: https://pr-9295-documentation.appa.pantheon.site
🛠️ Manage in Pantheon: https://dashboard.pantheon.io/site/2b30153f-e8b1-4427-b076-6109e704ba5d/overview

@@ -23,7 +23,72 @@ Pantheon backups are stored offsite on Google Cloud Storage instances. We recomm

<Download file="pantheon-backup-to-s3.sh" />

GITHUB-EMBED https://github.com/pantheon-systems/documentation/blob/main/source/scripts/pantheon-backup-to-s3.sh.txt bash GITHUB-EMBED
Copy link
Contributor

Choose a reason for hiding this comment

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

@rachelwhitton should we delete these .txt files too?

Copy link
Member Author

Choose a reason for hiding this comment

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

No because the download component expects it to be there for the button to work above the code snippet (used in cases where we are giving a full bash script for the user to run)
Screenshot 2024-11-07 at 1 37 59 PM


<Accordion title={"Full text of wp-config-local-sample.php"} id={"full-wp-config-local-sample"}>

GITHUB-EMBED https://github.com/pantheon-systems/WordPress/blob/default/wp-config-local-sample.php php GITHUB-EMBED
Copy link
Contributor

Choose a reason for hiding this comment

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

@rachelwhitton we should probably retain the link to this file, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's linked above on line 215

@@ -47,19 +47,153 @@ Read the comments in the scripts carefully to ensure that you modify variables c

<Download file="collect-logs-rsync.sh" />

GITHUB-EMBED https://github.com/pantheon-systems/documentation/blob/main/source/scripts/collect-logs-rsync.sh.txt shell:title=collect-logs-rsync.sh GITHUB-EMBED
Copy link
Contributor

Choose a reason for hiding this comment

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

Another .txt file to possibly delete

@@ -71,7 +205,7 @@ You can run the script with parameters for reusability.

1. Edit the 'collect-logs-rsync.sh' (you can choose to disable the AGGREGATE_NGINX):

```bash{promptUser:user}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this an unrelated fix? I'm fine with including it if it's generally beneficial.

Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated, just noticed it was used incorrectly as i was fixing other things on the page

Copy link
Contributor

@stevector stevector left a comment

Choose a reason for hiding this comment

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

@rachelwhitton I should review the build when it completes too but I've already added some questions/comments.

Copy link

⚡ Deployed with Pantheon Decoupled

This build was successfully deployed with Pantheon. You can track the build logs here.

👀 Preview: https://pr-9295-documentation.appa.pantheon.site
🛠️ Manage in Pantheon: https://dashboard.pantheon.io/site/2b30153f-e8b1-4427-b076-6109e704ba5d/overview

@stevector stevector self-requested a review November 7, 2024 21:02
@stevector stevector merged commit 78ca1a5 into main Nov 7, 2024
12 checks passed
@stevector stevector deleted the rm-gatsby-remark-github branch November 7, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Process: High Priority Issue is critical / needs immediate attention Topic: Security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants