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

docs: move ExEx book examples #11616

Merged
merged 4 commits into from
Oct 14, 2024

Conversation

caglaryucekaya
Copy link
Contributor

Closes #11581

Code examples in ExEx book chapters Remote and Tracking State are moved to Rust files. There are small fixes for the examples that weren't working, and a new file build.rs to make the proto file in the Remote example compile. Otherwise, the example wasn't working as explained here.

@shekhirin shekhirin changed the title enh: move exex book examples docs: move ExEx book examples Oct 9, 2024
@shekhirin shekhirin added C-docs An addition or correction to our documentation A-exex Execution Extensions labels Oct 9, 2024
Ok(())
}

// ANCHOR: snippet
Copy link
Collaborator

@shekhirin shekhirin Oct 9, 2024

Choose a reason for hiding this comment

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

do we need this? and in other places too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to show only the relevant sections of the files at the book references, and allow the reader to expand the complete file only if they want to. Another way to do this is to use line numbers of the snippets instead of using these // ANCHOR: snippet comments, but then we'd have to change the line numbers if the examples are changed. Or I could just show the complete files at all references. Which one do you prefer?

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, i see! that's great, I think these anchors are fine

@@ -0,0 +1,4 @@
fn main() -> Result<(), Box<dyn std::error::Error>> {
tonic_build::compile_protos("proto/exex.proto")?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

for this to work in the CI, we also need to install protoc. I think it's not a big deal as it's quite lightweight, wdyt @mattsse ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

refactoring the job and adding protoc in #11618

Copy link
Collaborator

Choose a reason for hiding this comment

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

why does this need to run on ci?
yeah I guess if we want to do have well maintained bindings then we're kinda forced to do this. sounds reasonable

Copy link
Collaborator

@shekhirin shekhirin left a comment

Choose a reason for hiding this comment

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

small nits, LGTM otherwise

book/sources/exex/tracking-state/src/bin/1.rs Outdated Show resolved Hide resolved
book/sources/exex/remote/src/exex.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@shekhirin shekhirin left a comment

Choose a reason for hiding this comment

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

Amazing, thanks!

@shekhirin shekhirin added this pull request to the merge queue Oct 14, 2024
Merged via the queue into paradigmxyz:main with commit b637101 Oct 14, 2024
39 checks passed
vandenbogart pushed a commit to testmachine-ai/reth that referenced this pull request Oct 14, 2024
Co-authored-by: Alexey Shekhirin <a.shekhirin@gmail.com>
reymom pushed a commit to reymom/reth that referenced this pull request Oct 15, 2024
Co-authored-by: Alexey Shekhirin <a.shekhirin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-exex Execution Extensions C-docs An addition or correction to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move ExEx book code examples to the separate Cargo workspace
3 participants