Skip to content

Conversation

@florian-lefebvre
Copy link
Member

@florian-lefebvre florian-lefebvre commented Apr 28, 2025

Changes

Testing

  • Updated tests so they still pass and TS is happy
  • Updated the orchestrate test to make sure init is well forwarded to the end

Docs

Changeset

@florian-lefebvre florian-lefebvre self-assigned this Apr 28, 2025
@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Apr 28, 2025
@changeset-bot
Copy link

changeset-bot bot commented Apr 28, 2025

🦋 Changeset detected

Latest commit: 690edae

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codspeed-hq
Copy link

codspeed-hq bot commented Apr 28, 2025

CodSpeed Performance Report

Merging #13705 will not alter performance

Comparing feat/fonts-update-unifont (690edae) with main (60d5be4)

Summary

✅ 6 untouched benchmarks

@florian-lefebvre florian-lefebvre marked this pull request as ready for review April 28, 2025 11:04
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Can we have a JSDoc and a return type for the orchestrate function? It's very big, we don't know what it does (orchestrate is a very generic term), and now we changed its return type, where we also changed internal variables, in which some comments were lost

@florian-lefebvre
Copy link
Member Author

The orchestrate function already has jsdocs explaining what happens inside. So to address your review, here is what i've done:

  1. I've specified an explicit return type
  2. I copied the comments from the types to the variables inside orchestrate

Because yes comments were not lost but moved to types.ts. Hope that helps but let me know if that's not enough, I'm happy to clarify anything!

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you! Apologies I missed the JSDoc attached to the function

@florian-lefebvre florian-lefebvre merged commit 28f8716 into main Apr 28, 2025
17 checks passed
@florian-lefebvre florian-lefebvre deleted the feat/fonts-update-unifont branch April 28, 2025 13:33
@astrobot-houston astrobot-houston mentioned this pull request Apr 28, 2025
openscript pushed a commit to openscript/astro that referenced this pull request Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants