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

Make first querybuilder example runnable #727

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

scotttrinh
Copy link
Collaborator

It was confusing to see an incomplete and non-runnable example in the middle of the documentation (we forgot to import the e default export), so I made the very first example a runnable example so that all of the following examples could assume a similar setup.

One sort-of awkward exception is the part where we're inferring the type with the $infer type helper. We show some module importing there too, but since we're not running any async code, we don't need a wrapping function. I wonder if we should show $infer in that first example, too, and just give more details in the "Extracting the inferred type" section? 🤔

@scotttrinh scotttrinh requested a review from raddevon September 7, 2023 00:25
@raddevon
Copy link
Contributor

I see what you mean. What if we made every code block runnable on its own? We'd have to repeat some imports, but it would be nice for readers to be able to have a consistent assumption about each one.

@scotttrinh
Copy link
Collaborator Author

What if we made every code block runnable on its own? We'd have to repeat some imports, but it would be nice for readers to be able to have a consistent assumption about each one.

Yeah, there is definitely some utility there, but to make it runnable, it needs the imports, the client instantiation, the async function, and the call to the async function for a total of 8 additional lines. Most of the existing code blocks are not even 8 lines as-is, so it would double or more most of the code blocks making it a bit less scannable. Seems noisy to me since you have to ignore most of the code block to actually see the feature that's being discussed making it a little less scannable.

I don't feel strongly about this, so happy to follow your lead here!

@raddevon
Copy link
Contributor

If we're going to lose either scannability or clarity here (which I think is the tradeoff, unless there's some third option that could fulfill both), I'd lean toward preserving clarity. I also don't feel super strongly about it, but that's where my intuition is leading me.

@scotttrinh
Copy link
Collaborator Author

@raddevon

I think my complaint with adding the boilerplate to each example is at least two-fold:

  1. Harder to scan
  2. Harder to correlate the code block with the topic being discussed

There are a few cases where the code blocks are only separated by a single sentence which seems to exacerbate the second issue.

Given that, I think maybe let's just stick with the change here and see if we get any more feedback that it's confusing. WDYT?

@raddevon
Copy link
Contributor

Sounds good. I'm on board!

@scotttrinh scotttrinh merged commit 108363a into master Sep 11, 2023
@scotttrinh scotttrinh deleted the docs/qb-runnable-example branch September 11, 2023 16:30
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.

2 participants