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

Fix upsert logic in check count #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix upsert logic in check count #64

wants to merge 1 commit into from

Conversation

ryoheinagao
Copy link

@ryoheinagao ryoheinagao commented Jun 6, 2022 •
edited
Loading

I found and fix that count has string type, so below line is always false.

node-mysql-utilities/utilities.js

Line 400 in e829aa1

if (count === 1) this.update(table, row, callback);

  • tests and linter show no problems (npm t)
    • Please tell me how to run test locally.
  • tests are added/updated for bug fixes and new features
  • code is properly formatted (npm run fmt)

@ryoheinagao ryoheinagao mentioned this pull request Jun 6, 2022
Open
@ryoheinagao ryoheinagao changed the title Fix upsert logic of check count Fix upsert logic in check count Jun 6, 2022
@ryoheinagao ryoheinagao marked this pull request as ready for review June 6, 2022 11:37
tshemsedinov
tshemsedinov approved these changes Jun 6, 2022
View reviewed changes
Copy link
Owner

@tshemsedinov tshemsedinov left a comment

Choose a reason for hiding this comment

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

LGTM

@ryoheinagao
Copy link
Author

Thank you for your quick review. Do I have to do anything else for merge and release?

@ryoheinagao
Copy link
Author

@tshemsedinov Can you tell me below?

Thank you for your quick review. Do I have to do anything else for merge and release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tshemsedinov tshemsedinov Awaiting requested review from tshemsedinov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ryoheinagao @tshemsedinov