-
Notifications
You must be signed in to change notification settings - Fork 28
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
Fix smoke tests parsing issue by adding smoke_tests.sql and husky pre… #60
base: main
Are you sure you want to change the base?
Fix smoke tests parsing issue by adding smoke_tests.sql and husky pre… #60
Conversation
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.
Do we need this file ?
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.
node_modules/husky/README.md
Outdated
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.
Do we need this file ?
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.
Do we need this file ?
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.
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.
Could you please add comments to your code? It would help others understand the functionality better.
Thank you @sanketdisale871 for your work on this pull request. The addition of a dedicated .sql file for smoke tests and the implementation of a husky pre-commit hook for SQL syntax validation are great improvements. To fully integrate these changes into our workflow, could you please also update our GitHub Actions workflow to use the new Please let me know if you need any assistance with this. Thanks again for your contribution. CC: @ChakshuGautam |
I need assistance. |
Sure @sanketdisale871 Instead of writing Here's a sample code
|
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.
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.
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.
All the SQL queries you have deleted from github/workflows/smoke-test.yml
should add back to this file.
Pull Request Description:
Fix Smoke Tests Parsing Issue
This pull request addresses issue #55.
Issue Description:
Currently, the smoke tests included in the workflow can lead to parsing issues when updates are made. This occurs due to the absence of a dedicated file for smoke tests and lack of validation for its SQL syntax.
Solution:
To resolve this issue, the following changes have been implemented:
The husky pre-commit hook ensures that the smoke_tests.sql file contains valid SQL syntax before committing any changes. This proactive validation helps to avoid parsing issues and ensures the integrity of the smoke tests.
Please review these changes and provide your feedback.