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

Adopt temporalio/snipsync for documentation #10768

Open
edmondop opened this issue Jun 2, 2024 · 5 comments
Open

Adopt temporalio/snipsync for documentation #10768

edmondop opened this issue Jun 2, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@edmondop
Copy link
Contributor

edmondop commented Jun 2, 2024

Is your feature request related to a problem or challenge?

Datafusion documentation is amazing and examples are too. However, inline snippets might fall out of sync from the codebase and are not very easy to consume because you often need to figure out the required imports to get a snippet to compile. On the other side, writing a full Rust script in a documentation page creates too much noise and make the documentation less effective.

Describe the solution you'd like

Ideally, we would have snippets that are portions of executable scripts, so that we can via CI/CD execute them and confirm they are not obsolete, but at the same time only the relevant section of the script is included in the docs.

We can use https://github.com/temporalio/snipsync for the purpose

Describe alternatives you've considered

No response

Additional context

I was working on a separate issue for documentation, and I have found myself facing the problem that I mentioned above, see #7306 (comment)

@edmondop edmondop added the enhancement New feature or request label Jun 2, 2024
@andygrove
Copy link
Member

It looks like an interesting solution. We definitely need something like this, and I suppose I had stared building my own tooling, which is probably a bad idea.

The snipsync repo does not seem very active or popular and uses npm and yarn which I am not sure many DataFusion contributors would be familiar with.

I wonder if there are other solutions out there?

@edmondop
Copy link
Contributor Author

edmondop commented Jun 7, 2024

There are other such as https://github.com/SimonCropp/MarkdownSnippets, but I know Temporal is a popular OSS product, with excellent documentation, see https://temporal.io/blog/snipsync-overview

So I guess there are these two options, and maintaining our tool

@alamb
Copy link
Contributor

alamb commented Aug 20, 2024

I believe we now achieve this goal using the doctest! macro -- for example

// Instructions for Documentation Examples
//
// The following commands test the examples from the user guide as part of
// `cargo test --doc`
//
// # Adding new tests:
//
// Simply add code like this to your .md file and ensure your md file is
// included in the lists below.
//
// ```rust
// <code here will be tested>
// ```
//
// Note that sometimes it helps to author the doctest as a standalone program
// first, and then copy it into the user guide.
//
// # Debugging Test Failures
//
// Unfortunately, the line numbers reported by doctest do not correspond to the
// line numbers of in the .md files. Thus, if a doctest fails, use the name of
// the test to find the relevant file in the list below, and then find the
// example in that file to fix.
//
// For example, if `user_guide_expressions(line 123)` fails,
// go to `docs/source/user-guide/expressions.md` to find the relevant problem.

@edmondop
Copy link
Contributor Author

@alamb there might still good reasons to consider snipsync or an approach such as the one that snipsync implements:

  • We do have examples in two places: in the user guide and indatafusion-examples. With snipsync apprach, they will all be in a single location and could be imported in the markdown
  • Using comments/tags, it would be possible to import in the markdown only a portion of the example by looking at comments, so that the reader can focus on the relevant part of the code

What do you think about modifying the macro to support these two features?

  • external example (specifying the path)
  • using comments in the rust code
use std::error::Error;
// Snipstart: "Snip1"
print!("foo");
// Snipend
print!("Some uninteresting code");
// Snipstart: "Errors"
panic!("Something went wrong");
// Snipend
You can print foo in Datafusion using the `print!` macro:
<!--include examples/example_1.rs snip-name="Snip1" -->

Another important feature in Datafusion is `panic!`, which can be used like so:
<!--include examples/example_1.rs snip-name="Errors" -->

@alamb
Copy link
Contributor

alamb commented Aug 20, 2024

What do you think about modifying the macro to support these two features?

I think in theory it sounds great. I have no strong preference on tool or methodology.

What I think is important is that it is as easy as possible to write / maintain the documentation + examples (clear docs on how to document things, good examples of doing so with the docs)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants