Skip to content

Conversation

@vikions
Copy link

@vikions vikions commented Jan 16, 2026

This PR moves the op-rbuilder primitives into the shared base-primitives crate, de-duplicating the Bundle type and aligning with the current builder structure.

Summary

  • Moved op-rbuilder primitives into crates/shared/primitives/src/op_rbuilder
  • Re-exported primitives via base-primitives and added op-rbuilder feature
  • Updated op-rbuilder imports to use base_primitives::op_rbuilder
  • De-duplicated Bundle in favor of the existing base-bundles crate
  • Gated OTLP telemetry behind the telemetry feature and updated call sites

Testing

  • cargo check -p op-rbuilder
  • cargo check -p op-rbuilder --features telemetry

This is a clean PR rebased on the latest main to avoid the conflicts in the previous one.
Closes #370

@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Jan 16, 2026

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

@danyalprout
Copy link
Collaborator

My take here is lets remove the feature flags (telemetary and op-rbuilder) and only move the bundle type for now. We can leave the execution type etc. in op-rbuilder

Comment on lines +50 to +59
#[cfg(feature = "telemetry")]
{
use crate::telemetry::setup_telemetry_layer;
let telemetry_layer = setup_telemetry_layer(&telemetry_args)?;
cli_app.access_tracing_layers()?.add_layer(telemetry_layer);
}
#[cfg(not(feature = "telemetry"))]
{
return Err(eyre::eyre!("telemetry feature is disabled"));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason to reintroduce this feature flag? I personally would rather avoid feature flags if possible.

base-builder-cli.workspace = true
base-bundles.workspace = true
base-flashtypes.workspace = true
base-primitives = { workspace = true, features = ["op-rbuilder"] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets remove the op-rbuilder feature flag on primitives. No need to gate the basic types

@vikions
Copy link
Author

vikions commented Jan 16, 2026

My take here is lets remove the feature flags (telemetary and op-rbuilder) and only move the bundle type for now. We can leave the execution type etc. in op-rbuilder

Hey! Just to clarify - when you say "only move the bundle type for now", should I revert the reth/ folder (engine_api_builder, execution types) back to op-rbuilder and keep only bundle.rs in shared?

I moved everything initially based on the issue saying "split out primitives", but if you want a smaller PR with just bundles for now, I can revert the reth stuff back. Let me know what you prefer!

@refcell
Copy link
Contributor

refcell commented Jan 16, 2026

My take here is lets remove the feature flags (telemetary and op-rbuilder) and only move the bundle type for now. We can leave the execution type etc. in op-rbuilder

Hey! Just to clarify - when you say "only move the bundle type for now", should I revert the reth/ folder (engine_api_builder, execution types) back to op-rbuilder and keep only bundle.rs in shared?

I moved everything initially based on the issue saying "split out primitives", but if you want a smaller PR with just bundles for now, I can revert the reth stuff back. Let me know what you prefer!

Apologies for the confusion @vikions

Yes, for now, let's only add bundle.rs in base-primitives and move the who op_rbuilder module stuff back to the op-rbuilder crate. Moving this stuff incrementally is a huge help, so it's totally ok if there are some primitives left in op-rbuilder that can be refactored/cleaned up further later

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.

chore(builder): Split Out Builder Primitives

4 participants