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

feat: meteor 3.0.4 #1314

Draft
wants to merge 3 commits into
base: release52
Choose a base branch
from
Draft

feat: meteor 3.0.4 #1314

wants to merge 3 commits into from

Conversation

Julusian
Copy link
Member

@Julusian Julusian commented Nov 4, 2024

About the Contributor

This pull request is posted on behalf of the BBC

Type of Contribution

This is a: Feature / Tech stack update

This isn't quite ready to merge yet, but I am opening this for visibility and to allow for early suggestions/reviews

New Behavior

This updates meteor to 3.0.4 and removes any remaining code which relies on fibers.

This PR includes the changes from #1308 and #1310, which are some more targeted changes which get certain areas of the codebase into a meteor 3 compatible state.

As part of this, everything has been updated to use and require node 20, which matches the version that meteor 3 is using. It looks like 3.1.0 might be using node 22.11, so we might be able to update to that in the not too distant future.

Devs should be aware that meteor doesn't seem to like meteor yarn anymore, but as we don't need to be tied to their exact version of node it is fine to invoke yarn directly.

I have looked over all the meteor packages we are using, and have stripped out some which are not necessary. Instead of needing a meteor specific elastic-apm library, we can now use the standard npm one. In doing this we loose some automatic integration with meteor, but we had most of this disabled and were only using it on collections where we already have a wrapper class making it easy to implement manually.

I have a discussion to start around this; the api of the old showstyle and studio migration system is very reliant on fibers. Before I put time into updating that, I would like to question whether it should be removed instead. It has been marked as deprecated for over 2 years now (since #808), so now that it has a measurable cost of needing some maintenance it feels like an appropriate time to question if it should be removed instead.

Testing

  • I have added one or more unit tests for this PR
  • I have updated the relevant unit tests
  • No unit test changes are needed for this PR

I have done some testing through configuring and using Sofie, and haven't found anything misbehaving.

Affected areas

Time Frame

Other Information

Status

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

@Julusian Julusian added the Contribution from BBC Contributions sponsored by BBC (bbc.co.uk) label Nov 4, 2024
Copy link

codecov bot commented Nov 4, 2024

Codecov Report

Attention: Patch coverage is 48.33210% with 697 lines in your changes missing coverage. Please review.

Project coverage is 57.62%. Comparing base (ea7cece) to head (421d906).
Report is 162 commits behind head on release52.

Files with missing lines Patch % Lines
...ver/collections/implementations/asyncCollection.ts 53.51% 119 Missing ⚠️
...eteor/server/api/deviceTriggers/triggersContext.ts 40.35% 68 Missing ⚠️
...eContentStatusUI/rundown/rundownContentObserver.ts 0.00% 62 Missing ⚠️
...eceContentStatusUI/bucket/bucketContentObserver.ts 0.00% 50 Missing ⚠️
...packageManager/expectedPackages/contentObserver.ts 0.00% 42 Missing ⚠️
...erver/api/deviceTriggers/RundownContentObserver.ts 17.14% 29 Missing ⚠️
meteor/server/api/profiler/apm.ts 51.92% 25 Missing ⚠️
meteor/server/api/rest/koa.ts 4.00% 24 Missing ⚠️
meteor/server/publications/lib/debounce.ts 83.72% 21 Missing ⚠️
...ver/publications/partsUI/rundownContentObserver.ts 0.00% 18 Missing ⚠️
... and 56 more
Additional details and impacted files
@@              Coverage Diff              @@
##           release52    #1314      +/-   ##
=============================================
- Coverage      57.91%   57.62%   -0.30%     
=============================================
  Files            525      401     -124     
  Lines          84945    70568   -14377     
  Branches        4438     4564     +126     
=============================================
- Hits           49200    40662    -8538     
+ Misses         35689    29625    -6064     
- Partials          56      281     +225     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution from BBC Contributions sponsored by BBC (bbc.co.uk)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant