Skip to content

Conversation

lennartkloock
Copy link
Member

@lennartkloock lennartkloock commented Sep 23, 2025

This PR adds a new binary that executes both the email and the core service. A lot of code here is copied from the other binaries.

@lennartkloock lennartkloock requested review from a team as code owners September 23, 2025 19:24
Copy link

codecov bot commented Sep 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.79%. Comparing base (f91f96b) to head (7f6c024).
⚠️ Report is 127 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #597      +/-   ##
==========================================
- Coverage   61.99%   61.79%   -0.20%     
==========================================
  Files         381      381              
  Lines       30871    30970      +99     
==========================================
  Hits        19138    19138              
- Misses      11733    11832      +99     

see 4 files with indirect coverage changes

Components Coverage Δ
scuffle-aac 89.47% <ø> (ø)
scuffle-amf0 90.31% <ø> (ø)
scuffle-av1 98.40% <ø> (ø)
scuffle-batching 100.00% <ø> (ø)
scuffle-bootstrap 84.16% <ø> (ø)
scuffle-bytes-util 96.70% <ø> (ø)
scuffle-context 100.00% <ø> (ø)
scuffle-expgolomb 100.00% <ø> (ø)
scuffle-ffmpeg 90.00% <ø> (ø)
scuffle-flv 95.59% <ø> (ø)
scuffle-future-ext 50.00% <ø> (ø)
nutype-enum ∅ <ø> (∅)
scuffle-h264 99.68% <ø> (ø)
scuffle-http 86.21% <ø> (ø)
scuffle-metrics 95.02% <ø> (ø)
postcompile 44.24% <ø> (ø)
scuffle-pprof 100.00% <ø> (ø)
scuffle-rtmp 91.16% <ø> (ø)
scuffle-settings 99.11% <ø> (ø)
scuffle-signal 95.41% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment on lines 238 to 242
let email_service_channel = tonic::transport::Channel::from_shared(email_service_address)
.context("create channel to email service")?
.connect_lazy();
let email_service_client =
pb::scufflecloud::email::v1::email_service_client::EmailServiceClient::new(email_service_channel);
Copy link
Member

Choose a reason for hiding this comment

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

I think, i dont like this. In theory we should be able to somehow use the actual service itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean without networking?

Copy link
Member

Choose a reason for hiding this comment

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

yeah

Copy link
Contributor

github-actions bot commented Sep 23, 2025

🚀 Preview Deployments

Deployment Status Preview URL
dashboard https://app-scuffle-cloud-ljno3c6q7-scufflecloud.vercel.app
docs ⏭️ -
rustdoc https://docs-scuffle-24lxjk2lr-scufflecloud.vercel.app
cloud emails render https://scufflecloud-email-preview-i7mlsiy9y-scufflecloud.vercel.app

@SimaoMoreira5228
Copy link
Contributor

Why combine the email and core services into a single binary? What are the advantages of this approach over using them as separate microservices?

@lennartkloock
Copy link
Member Author

Why combine the email and core services into a single binary? What are the advantages of this approach over using them as separate microservices?

Primarily for development right now, it's much simpler to just run one binary instead of having to start a few binaries in a specific order. Especially when we add even more services later.

@SimaoMoreira5228
Copy link
Contributor

Primarily for development right now, it's much simpler to just run one binary instead of having to start a few binaries in a specific order. Especially when we add even more services later.

That makes sense. It's simpler to run and build one binary, especially as we add more services.

@lennartkloock lennartkloock marked this pull request as draft September 23, 2025 22:08
@lennartkloock
Copy link
Member Author

Stale because of the comment above #597 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants