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.

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

Closes #2 by changing dotenv variable name #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

markf94
Copy link

@markf94 markf94 commented Jan 8, 2021

First of all, thanks @Mosuswalks for this amazing project! I really like the simplicity and elegance of it 👏 excited to contribute to it!

This MR closes issue #2 by changing the variable name retrieved from the .env file from CV_CODE to CVV as outlined by @sjh37 in the issue. However, I decided to go with CVV (Credit Verification Value) instead of CVC to avoid confusion or accidental mistakes since it is called CVV.

On top of that, I found the doc string for the size variable slightly confusing since it did not clear to me that the size string should be either "US M 8.5" or "US W 11" and not "US M 8.5 / W 11". I tried to make that more clear in the additional comment line.

As described in Mosuswalks#2, the README tells the user to create a `.env` file
with a `CVC` variable in it but the `bot.js` file references a `CV_CODE`
variable in the dotenv file which doesn't exists. It's actually called a
Card Verification Value (CVV) code and not a CVC code and hence the
variable names were adjusted to match that.
to make it clear that the size should be either 'US M 9.5' or 'US 12' or
'W 10' and not 'US M 9.5 / W 11' (which isn't a valid size).
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.

1 participant