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.

Sign up for GitHub

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

Jump to bottom

docs: add example for field_references #520

Merged
merged 4 commits into from
Nov 22, 2023
Merged

docs: add example for field_references #520

merged 4 commits into from
Nov 22, 2023

Conversation

MasseGuillaume
Copy link
Contributor

@MasseGuillaume MasseGuillaume commented Jul 13, 2023 •
edited
Loading

@github-actions
Copy link

ACTION NEEDED

Substrait follows the Conventional Commits
specification for
release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

@MasseGuillaume MasseGuillaume force-pushed the field-reference-example branch from f0bd302 to 62406b2 Compare July 13, 2023 16:30
@MasseGuillaume MasseGuillaume changed the title doc: add example for field_references docs: add example for field_references Jul 13, 2023
@vbarua vbarua mentioned this pull request Jul 13, 2023
Merged
@MasseGuillaume MasseGuillaume force-pushed the field-reference-example branch from 5a1c2b1 to 96caf2e Compare July 13, 2023 23:52
westonpace
westonpace previously requested changes Jul 20, 2023
View reviewed changes
Copy link
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

This example is currently just dropped in without any explanation or motivation. Can we add a paragraph before it explaining why there is an example here?

image

The masked complex expression section is a good example

image

@EpsilonPrime EpsilonPrime added awaiting-user-input This issue is waiting on further input from users and removed awaiting-user-input This issue is waiting on further input from users labels Oct 12, 2023
@EpsilonPrime
Copy link
Member

@westonpace It looks like changes were made based on your feedback. Could you take a second look please? Thanks!

@EpsilonPrime EpsilonPrime self-requested a review as a code owner November 22, 2023 19:10
@EpsilonPrime EpsilonPrime dismissed westonpace’s stale review November 22, 2023 19:13

Change reviewed by enough SMC members.

@EpsilonPrime EpsilonPrime merged commit 9f2a09c into substrait-io:main Nov 22, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vbarua vbarua vbarua left review comments

@jacques-n jacques-n jacques-n approved these changes

@EpsilonPrime EpsilonPrime EpsilonPrime approved these changes

@cpcloud cpcloud Awaiting requested review from cpcloud cpcloud is a code owner

@westonpace westonpace Awaiting requested review from westonpace westonpace is a code owner

Assignees

@westonpace westonpace

Labels
awaiting SMC approval
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MasseGuillaume @EpsilonPrime @jacques-n @vbarua @westonpace