Skip to content

Fix integration script for non-core adapters#4741

Merged
matthewp merged 12 commits into
mainfrom
move-deno-netlify-out-of-core
Sep 27, 2023
Merged

Fix integration script for non-core adapters#4741
matthewp merged 12 commits into
mainfrom
move-deno-netlify-out-of-core

Conversation

@matthewp
Copy link
Copy Markdown
Contributor

@matthewp matthewp commented Sep 18, 2023

What kind of changes does this PR include?

  • Changes to the docs site code

Description

  • Removes @astrojs/netlify and @astrojs/deno from docs generation as those integrations are no longer controlled by core. They will be documented in their own repository, like with any other 3rd party package.

Related Issue / Implementation PR

@netlify
Copy link
Copy Markdown

netlify Bot commented Sep 18, 2023

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit c59505c
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/651477f64bebf900085d41ba
😎 Deploy Preview https://deploy-preview-4741--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 configuration.

@matthewp
Copy link
Copy Markdown
Contributor Author

From discussion in Discord, the outstanding thing here is to figure out what to do about the pages that are now excluded. Some options:

  • Set up redirects to the new repository homes.
  • Add a mdx file that explains that they are now maintained externally with a link.

Comment thread scripts/generate-integration-pages.ts Outdated
@sarah11918 sarah11918 added site improvement Some thing that improves the website functionality - ask @delucis for help! merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) labels Sep 20, 2023
@sarah11918
Copy link
Copy Markdown
Member

Note to us to merge this when the affected repositories have been moved, and we'll make any necessary docs changes to content on our end at that time.

(E.g. if there is no page remaining to pull in, our content will simply not update and continue to show what's current, so we'll have to either remove manually on our end, or update manually to show a redirect etc.)

@matthewp matthewp force-pushed the move-deno-netlify-out-of-core branch from ae866f0 to 4584e1e Compare September 20, 2023 15:04
Comment thread src/content/docs/en/guides/integrations-guide/deno.mdx Outdated
Comment thread src/content/docs/en/guides/integrations-guide/deno.mdx Outdated
matthewp and others added 2 commits September 20, 2023 13:41
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Comment thread src/content/docs/en/guides/integrations-guide/deno.mdx Outdated
Comment thread scripts/generate-integration-pages.ts
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

I'm approving the page text! This should wait for Chris to confirm some page generation questions I had.

Comment thread scripts/generate-integration-pages.ts
Comment thread src/content/docs/en/guides/integrations-guide/deno.mdx Outdated
The Deno adapter allows Astro to deploy your SSR site to Deno targets including Deno Deploy.

[astro-integration]: /en/guides/integrations-guide/
The Deno adapter was previously maintained by Astro but now is maintained by Deno directly. Usage is now documented [in the Deno adapter repository](https://github.com/denoland/deno-astro-adapter).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I’d be in favour of minimal migration docs here once we know the new package name. Something along the lines of:

  1. Uninstall @astrojs/deno
  2. Install deno-astro-adapter (or whatever it ends up being called)
  3. Update the import statement in astro.config.mjs to use the new package:
    - import deno from '@astrojs/deno';
    + import deno from 'deno-astro-adapter';

But I can understand if we’re reticent to provide any guidance or want to keep that somewhere else.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do like this! Let's do it!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we're going to want to merge this before they have released a new adapter (which we don't know when is going to happen). So would it be ok to go with this for now and update the guidance once that's out?

Comment thread src/content/docs/en/guides/integrations-guide/deno.mdx
@matthewp
Copy link
Copy Markdown
Contributor Author

The home for Netlify might be changing, so holding off on this one for now, putting back as a draft.

@matthewp matthewp marked this pull request as draft September 25, 2023 19:43
@matthewp
Copy link
Copy Markdown
Contributor Author

Ok, this is ready for review again. This is cleaned up so that only Deno is removed from generation. Netlify is now maintained in https://github.com/withastro/adapters/tree/main/packages/netlify and the plan is that other adapters will be moved over. I'll update the script again when those are moved over.

@matthewp matthewp marked this pull request as ready for review September 26, 2023 20:34
Copy link
Copy Markdown
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 @matthewp! Left a couple of notes.

Also underlining that these changes mean Deno will no longer show up in our integrations nav:

SSR adapters navigation showing only Cloudflare, Netlify, Node, and Vercel adapters

I think that was clear from our discussion, but might be worth us checking where and how we want to highlight its existence in the future, so that people who want it can still find it.

Comment thread scripts/generate-integration-pages.ts
Comment thread src/content/docs/en/guides/integrations-guide/deno.mdx Outdated
Comment thread src/content/docs/en/guides/integrations-guide/deno.mdx Outdated
@sarah11918
Copy link
Copy Markdown
Member

Following @delucis comment above re: now Deno doesn't show up on our list, I updated the SSR page with links to both the integrations directory on astro.build and to our official integrations guide page in docs.

Copy link
Copy Markdown
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.

Looks good — thanks @matthewp!

@matthewp matthewp merged commit a37a97a into main Sep 27, 2023
@matthewp matthewp deleted the move-deno-netlify-out-of-core branch September 27, 2023 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) site improvement Some thing that improves the website functionality - ask @delucis for help!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants