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-5238 Clone docs-laravel submodule for Netlify deploys #52

Merged
merged 26 commits into from
Dec 17, 2024

Conversation

anabellabuckvar
Copy link
Collaborator

@anabellabuckvar anabellabuckvar commented Dec 16, 2024

TICKET

DOP-5238 Clone docs-laravel submodule for Netlify deploys

NOTES

The docs-laravel repo has a submodule that is necessary for it to build properly. The repo already has a special build script for the docs-laravel Netlify site. This PR updates the laravel submodule and therefore enables docs-laravel to build on frontend-sourced Netlify sites(docs-frontend-stg, docs-frontend-dotcomstg, docs-frontend-dotcomprd)

The submodule is cloned in the same way as in this build script: https://github.com/10gen/docs-laravel/blob/v4.x/build.sh

STAGING LINKS

docs-frontend-dotcomstg laravel permalink. Deployed with netlify-test-deploy on Slack
docs-frontend-stg laravel permalink Deployed manually on netlify-testing-2 Snooty frontend branch

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for site-links-display failed. Why did it fail? →

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/site-links-display/deploys/6761c3062240390008f9bdf4

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for test-monorepo-redoc failed. Why did it fail? →

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/test-monorepo-redoc/deploys/6761c306c2d7740009cf2a28

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for slack-deploy-extension canceled.

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/slack-deploy-extension/deploys/6761c3061038770008f0835b

Copy link

netlify bot commented Dec 16, 2024

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

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/git-changed-file-extension/deploys/6761c306b80a0e000894376d

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for search-manifest-extension failed. Why did it fail? →

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/search-manifest-extension/deploys/6761c3060474bf00087c61b0

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for snooty-cache-extension canceled.

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/snooty-cache-extension/deploys/6761c306e1f1670008649fa6

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for persistence-module-ext canceled.

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/persistence-module-ext/deploys/6761c306e1f1670008649f9d

Copy link

netlify bot commented Dec 16, 2024

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

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/redirects-and-publish-extension/deploys/6761c30631e51a00089ae626

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for populate-data-extension ready!

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/populate-data-extension/deploys/6761c3060474bf00087c61ac
😎 Deploy Preview https://deploy-preview-52--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 Dec 16, 2024

Deploy Preview for redoc-ext-test canceled.

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/redoc-ext-test/deploys/6761c306e1f1670008649fa1

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for search-manifest-integration failed. Why did it fail? →

Name Link
🔨 Latest commit 77a08ce
🔍 Latest deploy log https://app.netlify.com/sites/search-manifest-integration/deploys/6761c306f09f15000878227a

@anabellabuckvar anabellabuckvar changed the title DOP-5238 add conditional for cloning docs-laravel repo DOP-5238 Clone docs-laravel submodule for Netlify deploys Dec 16, 2024
@anabellabuckvar anabellabuckvar marked this pull request as ready for review December 16, 2024 20:58

// Docs-laravel requires a submodule to build
if (repoName === 'docs-laravel') {
process.chdir(`${repoName}`);
Copy link
Collaborator

@i80and i80and Dec 17, 2024

Choose a reason for hiding this comment

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

It's typically better to pass in the cwd that you want into run.command, e.g. I think this is right: process.run(..., {cwd: reponame}).

process.chdir() will probably work for this purpose, but is an antipattern especially in async or threaded code because it's global process-wide state. While each task is running, and if any of these invocations throw, the process is in a bit of a weird liminal state.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for pointing that out and explaining! Implemented changes accordingly

extensions/populate-metadata/src/updateConfig.ts Outdated Show resolved Hide resolved
extensions/populate-metadata/src/updateConfig.ts Outdated Show resolved Hide resolved
@anabellabuckvar anabellabuckvar merged commit 9d4336a into main Dec 17, 2024
29 of 45 checks passed
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