Skip to content

Improve documentation on verified_* testing methods #828

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

Merged
merged 1 commit into from
Mar 7, 2023

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Mar 6, 2023

As I have been reviewing PRs and working on #827 I have always wanted slightly more specific documentation on the testing methods

@coveralls
Copy link

Pull Request Test Coverage Report for Build 4345790591

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 86.187%

Totals Coverage Status
Change from base Build 4344757158: 0.0%
Covered Lines: 13565
Relevant Lines: 15739

💛 - Coveralls

Copy link
Contributor

@ankrgyl ankrgyl left a comment

Choose a reason for hiding this comment

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

Wow, thank you for writing this up!

A wishlist item for me would be a quick guide (even just bullet points) that defines what tests you should add for your change (e.g. add a sanity test, a roundtrip test, etc.).

@alamb
Copy link
Contributor Author

alamb commented Mar 7, 2023

A wishlist item for me would be a quick guide (even just bullet points) that defines what tests you should add for your change (e.g. add a sanity test, a roundtrip test, etc.).

Yes, that would be great. If you have time that would be great. Maybe we could add it to the contributing section of the README https://github.com/sqlparser-rs/sqlparser-rs/blob/main/README.md#contributing

@alamb alamb merged commit 9ea396d into apache:main Mar 7, 2023
@alamb alamb deleted the alamb/docs branch April 9, 2024 20:32
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.

3 participants