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

Astro rewrite #53

Merged
merged 74 commits into from
Mar 8, 2024
Merged

Astro rewrite #53

merged 74 commits into from
Mar 8, 2024

Conversation

florian-lefebvre
Copy link
Collaborator

@florian-lefebvre florian-lefebvre commented Dec 28, 2023

Here it is, finally! Here are the final tasks you need to tackle before we can merge this in:

  • Please check for any visual regression
  • Make sure the newsletter endpoint works
  • Make sure the cli works
  • SEO check
  • Lighthouse check (requires preview deployment)
  • Make sure GH Actions still work
  • Make sure there is no issue with the cloudflare adapter (I've never used it before)
  • Make sure I didn't forget any content
  • Cloudflare compile time
  • Sentry

Note: sentry is not fully configured for a few reasons:

@alexanderniebuhr question about the cloudflare adapter and images. I had a look at the docs and I'm unsure about what we should do. The only way to benefit from image optimization as of now is to use the cloudflare imageService right? For now, I've specified passthrough.

Reviews:

@florian-lefebvre
Copy link
Collaborator Author

@anonrig What is the error you're getting on cloudflare?

@anonrig
Copy link
Owner

anonrig commented Jan 23, 2024

@anonrig What is the error you're getting on cloudflare?

Unfortunately, Cloudflare doesn't allow different build configs for production and preview branches, therefore it's looking at Next.js build command instead of Astro right now.

@florian-lefebvre
Copy link
Collaborator Author

Then if it's building locally for you, I think it's pretty safe to merge 🤞

@anonrig
Copy link
Owner

anonrig commented Jan 23, 2024

Then if it's building locally for you, I think it's pretty safe to merge 🤞

Perfect! Thank you for your work @florian-lefebvre. I'm still waiting for Biome and IntelliJ support for Astro. Unfortunately, the ecosystem still lacks to support Astro.

@florian-lefebvre
Copy link
Collaborator Author

You're welcome, that was interesting! And yeah definitely, VSC + Prettier have better support for sure

@anonrig
Copy link
Owner

anonrig commented Feb 27, 2024

Since, Zed and Biome now supports Astro, we can land this PR. @florian-lefebvre Would you mind updating the PR with the changes of main branch contents?

Copy link

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSource
Install scripts npm/workerd@1.20231030.0
Install scripts npm/workerd@1.20240304.0

View full report↗︎

Next steps

What is an install script?

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

  • @SocketSecurity ignore npm/workerd@1.20231030.0
  • @SocketSecurity ignore npm/workerd@1.20240304.0

@anonrig anonrig merged commit 1009fce into main Mar 8, 2024
4 of 5 checks passed
@anonrig anonrig deleted the astro-rewrite branch March 8, 2024 18:44
@florian-lefebvre
Copy link
Collaborator Author

I swear I didn't receive the notification from last week or discarded it by mistake! Otherwise I would have done it, sorry about that!

@anonrig
Copy link
Owner

anonrig commented Mar 8, 2024

No worries! You already did a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to Astro
3 participants