-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Minor: Document how to test for trailing whitespace in slt
/ sqllogictests
#13215
Conversation
slt
/ sqllogictests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
datafusion/sqllogictest/README.md
Outdated
``` | ||
|
||
To test trailing whitespace, project additional non-whitespace column on the | ||
right. For example, by selecting `'XX'` after the column of interest, the test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's any non-whitespace, but we can take opportunity to suggest what we want to see in tests.
What about '|'
that's used in one test i added, or '$'
(not non-typical for marking end of line)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use |
to be consistent with what you added . I will update the PR
Co-authored-by: Piotr Findeisen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This lgtm!
…ictests (apache#13215) * Minor: Document how to test for trailing whitespace * Apply suggestions from code review Co-authored-by: Piotr Findeisen <[email protected]> * Use `|` for consistency --------- Co-authored-by: Piotr Findeisen <[email protected]>
Which issue does this PR close?
Rationale for this change
@findepi figured out a good way to test for trailing spaces, let's put that in the user documentation as suggested by #13197 (comment)
What changes are included in this PR?
Add docs and slt examples
Are these changes tested?
Yes
Are there any user-facing changes?
docs and tests, no functional changes