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

DOP-5317: Populate-metadata extension supports content-repo-sourced deployments #64

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

anabellabuckvar
Copy link
Collaborator

@anabellabuckvar anabellabuckvar commented Jan 21, 2025

Ticket

DOP-5317 Populate-metadata extension supports content-repo-sourced deployments

Notes

Changes how populate-metadata netlify extension populates build environment env variables,. to support the transition to content-repo sourced deploys in dotcomstg, dotcomprd

Deployed version of populate-metadata extension

Docs build

@anabellabuckvar anabellabuckvar marked this pull request as ready for review January 21, 2025 19:54
Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for redoc-extension canceled.

Name Link
🔨 Latest commit 94136b6
🔍 Latest deploy log https://app.netlify.com/sites/redoc-extension/deploys/6793ab2ec590210008ef4860

Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for snooty-cache-extension canceled.

Name Link
🔨 Latest commit 94136b6
🔍 Latest deploy log https://app.netlify.com/sites/snooty-cache-extension/deploys/6793ab2ee6a3a50008c143af

Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for populate-data-extension ready!

Name Link
🔨 Latest commit 94136b6
🔍 Latest deploy log https://app.netlify.com/sites/populate-data-extension/deploys/6793ab2e4e57920008e7a8b0
😎 Deploy Preview https://deploy-preview-64--populate-data-extension.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 configuration.

Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for git-changed-file-extension canceled.

Name Link
🔨 Latest commit 94136b6
🔍 Latest deploy log https://app.netlify.com/sites/git-changed-file-extension/deploys/6793ab2e7633e70008724385

Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for redirects-and-publish-extension canceled.

Name Link
🔨 Latest commit 94136b6
🔍 Latest deploy log https://app.netlify.com/sites/redirects-and-publish-extension/deploys/6793ab2e8fe77b0008ebebe9

Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for search-manifest-extension canceled.

Name Link
🔨 Latest commit 94136b6
🔍 Latest deploy log https://app.netlify.com/sites/search-manifest-extension/deploys/6793ab2ef2e8d10008e237dc

Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for persistence-module-ext canceled.

Name Link
🔨 Latest commit 94136b6
🔍 Latest deploy log https://app.netlify.com/sites/persistence-module-ext/deploys/6793ab2e5650b50008fb44cc

Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for slack-deploy-extension canceled.

Name Link
🔨 Latest commit 7abd3de
🔍 Latest deploy log https://app.netlify.com/sites/slack-deploy-extension/deploys/679023d8ddec90000832bb01

@anabellabuckvar anabellabuckvar changed the title DOP-5317 change how environments are configured DOP-5317: Populate-metadata extension supports content-repo-sourced deployments Jan 24, 2025
Copy link
Collaborator

@seungpark seungpark left a comment

Choose a reason for hiding this comment

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

overall looks good! minor comments on removed logic and a follow up ticket for same file pathing configs

@@ -84,26 +87,18 @@ const cloneContentRepo = async ({
branchName: string;
orgName: string;
}) => {
if (fs.existsSync(repoName)) {
if (fs.existsSync(`${repoName}`)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor. is this necessary to interpolate here

await run.command(`rm -r ${repoName}`);
}

await run.command(
`git clone -b ${branchName} --recurse-submodules https://${process.env.GITHUB_BOT_USERNAME}:${process.env.GITHUB_BOT_PWD}@github.com/${orgName}/${repoName}.git -s`,
`git clone -b ${branchName} https://${process.env.GITHUB_BOT_USERNAME}:${process.env.GITHUB_BOT_PWD}@github.com/${orgName}/${repoName}.git -s`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

qq - why did we get rid of this option --recurse-submodules

);

// Remove git config as it stores the connection string in plain text
if (fs.existsSync(`${repoName}/.git/config`)) {
await run.command(`rm -r ${repoName}/.git/config`);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

is this not necessary anymore?

await run.command(`rm -r ${repoName}`);
}

await run.command(
`git clone -b ${branchName} --recurse-submodules https://${process.env.GITHUB_BOT_USERNAME}:${process.env.GITHUB_BOT_PWD}@github.com/${orgName}/${repoName}.git -s`,
`git clone -b ${branchName} https://${process.env.GITHUB_BOT_USERNAME}:${process.env.GITHUB_BOT_PWD}@github.com/${orgName}/${repoName}.git -s`,
);

// Remove git config as it stores the connection string in plain text
Copy link
Collaborator

Choose a reason for hiding this comment

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

also wondering if we can replace the contents of said doc to ./ folder of runner so that the structure is the same as a docs content repo.

this will be saving as:

| snooty
| -- cloud-docs
| ---- source
| ------ index.html

whereas the builds on docs sites have the structure:

| cloud-docs
| -- source
| ---- index.html
| -- snooty

we can make this a follow up ticket to keep the file structure the same

: (process.env.BRANCH_NAME ?? (configEnvironment.BRANCH as string));
const repoName =
process.env.REPO_NAME ??
(process.env.REPOSITORY_URL?.split('/')?.pop() as string);
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor. should we be reading from configEnvironment here as well

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.

2 participants