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

Foundation deployment #40

Merged
merged 15 commits into from
Dec 11, 2023
Merged

Foundation deployment #40

merged 15 commits into from
Dec 11, 2023

Conversation

microbit-matt-hillsdon
Copy link

@microbit-matt-hillsdon microbit-matt-hillsdon commented Dec 5, 2023

I've deleted the upstream workflows rather than disable them for clarity. Will always be easy to
reinstate if needed.

Non-trivial aspects:

  • The review/branch/PR previews model we use relies on a working base URL. There a few barriers to this:
    • A bespoke router redirects you to the root of the review domain so breaking refresh. I've added support for a base URL.
    • Absolute image paths. You can import images and have the bundler deal with them properly. This might be a bit disruptive though so perhaps we could do upstream
      • I've done it here for now. Quite a few images are in new UX.
    • Font paths. They're relative in the all.min.css source so not entirely clear what's going wrong.
      • They work by fluke upstream. In current use all.min.css is deployed to the root but references the fonts with ../webfonts/... this works because ../ at the root is ignored but at any other path it breaks. Maybe it was always intended to have a CSS directory? I've done that for now.
  • There's lots in public/ that will need to have suboptimal cache settings as the bundler isn't aware so output file names don't include a hash. Perhaps one to follow up on before we go live.
  • I've moved the 3D model from public/assets/model to just model as it wasn't managed by the bundler so can't get the same cache settings as other files in assets. public/assets is probably just not a good idea.

I've deleted the upstream workflows rather than
disable them for clarity. Will always be easy to
reinstate if needed.
Copy link

cloudflare-workers-and-pages bot commented Dec 5, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 44c770b
Status: ✅  Deploy successful!
Preview URL: https://d969778a.ml-trainer.pages.dev
Branch Preview URL: https://deployment.ml-trainer.pages.dev

View logs

@microbit-matt-hillsdon
Copy link
Author

Not worrying too much about the conflicts here for now. It might make sense for me to redo once current image changes are finalised.

- Vite base URL now works
- Images are hashed for deployment so can be cached
- Ignore images in src for the licensing check
@microbit-matt-hillsdon
Copy link
Author

The latest build is the first that isn't obviously broken: https://review-ml.microbit.org/deployment/

Will properly test the flows tomorrow.

Assets is a Vite managed folder - the model files are not hashed so not
safe to cache unlike everything else

Not easy to convert to be imported as one file references the other.
There's probably a Vite plugin that could do the job but one for
another time.
@microbit-matt-hillsdon
Copy link
Author

It works at least as well as main (radio bridge flow is broken, a bunch of images are placeholders).

@microbit-matt-hillsdon microbit-matt-hillsdon changed the title [WIP] Deployment Foundation deployment Dec 11, 2023
@microbit-matt-hillsdon microbit-matt-hillsdon marked this pull request as ready for review December 11, 2023 14:23
Copy link
Collaborator

@micque01 micque01 left a comment

Choose a reason for hiding this comment

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

LGTM, tested and it works nicely

@microbit-matt-hillsdon microbit-matt-hillsdon merged commit bff6c37 into main Dec 11, 2023
2 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