-
Notifications
You must be signed in to change notification settings - Fork 21
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
Bulk insert of failed receipts using query macro #329
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.
I think this is the last review. It has been a great work. Looking forward to fix these comments and merge it.
One suggestion: don't close and open new PRs unless they are have totally different code from each other. I see that you created PRs #326, #327, #328 and now #329 but they are all pointing to the same branch with one extra commit in each of them. It's hard to keep track of suggestions in other PRs.
Please take a look at the latest commit, hope everything's good |
Pull Request Test Coverage Report for Build 11119033601Details
💛 - Coveralls |
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 all looks good to me. Could you do a rebase on your side to update the commit messages so it passes our CI? This is a guide on how to do your commit messages.
Actually, I can do that on my side. Thank you for the great work and thanks for your contribution! |
Thanks a lot for the support sir, hoping to contribute more to the project |
Fixed #273
Added error logs storing to the table
Successfully ran commands:-
cargo clippy --all-targets --all-features
cargo fmt
cargo sqlx prepare --workspace -- --all-targets
please do recheck on your device