feat/bcrypt: add bcrypt command #201
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #201 +/- ##
=======================================
Coverage 78.93% 78.93%
=======================================
Files 26 26
Lines 1619 1619
=======================================
Hits 1278 1278
Misses 341 341 ☔ View full report in Codecov by Sentry. |
|
Is this still a draft, or ready to review? |
ready for review |
src/services/bcrypt/index.js
Outdated
| const salt = bc.genSaltSync(10); | ||
| const hashedText = bc.hashSync(text, salt); |
There was a problem hiding this comment.
are there any reasons behind the use of the sync version instead of its async counterpart?
There was a problem hiding this comment.
just to write simple code
There was a problem hiding this comment.
I'm not sure if bc.genSaltSync(10) is any simpler than just doing await bc.genSalt(10)
There was a problem hiding this comment.
got it, i'll update it to use await bc.genSalt(10)
|
Hi @zqwirp! Your bcrypt implementation causes an error that makes the bot keeps restarting over and over again. You can find the detail of the error here: https://teknologi-umum.sentry.io/share/issue/34a257c81f3c45a1b28b7b36aa1301b4/ On production, we're using Docker as defined on the Dockerfile (you can see it here https://github.com/teknologi-umum/bot/blob/master/Dockerfile). Any inputs are welcome. |
|
is it possible to revert this PR and let the existing command work as usual? ^-^ |
I've added the requested bcrypt command as explained in the issue. The
/bcryptcommand deletes the command and sends the hashed result of the command argument to the chat.Please review the PR and let me know if anything needs to be added or adjusted.