load username, email, and orcid number into config files #62
Conversation
sbillinge
changed the title
There was a problem hiding this comment.
Great start, thanks Steven. You did a really good job here! Here are some comments.
- please see how I edited the PR title. Your commit message is good but do not use a commit measage for a PR title, they do a different job.
- for the issues that this closes (based on the title) I think there will be two or three issues, not just one you will need a closes
#for each one. - per our discussion, there will not be a separate function for this task but it will be handled in the function that handles the flow. this is a difficult PR to atm because we didn't resolve that conversation, but I can still give feedback on what you did, so that is great!
- you have three tests but there is some redundancy in that multiple tests test the same thing (user enters username and email but not orcid at the command line)
- .please see inline comments.
- for this problem the more interesting tests will be the ones that don't handle entry on the command line. they will be a bit harder to write too.
But this is a good start. As I said, this is a slightly harder PR, so please maybe open the other ones you are working as well and I can give feedback on those too.
| actual_args = get_args(inputs) | ||
| # load username, email, orcid(optional)into config in key-value pairs | ||
| actual_args = load_username_email(actual_args) | ||
| assert actual_args == expected |
There was a problem hiding this comment.
this is not quite correct. your expected is a list of lists, but it should be args attributes.
| ["Please provide valid orcid number."], | ||
| ), | ||
| (["--username", "yucongalicechen%$#", "--email", "[email protected]"], ["Please provide valid username."]), | ||
| (["--username", "sbllinge", "--email", "sb2896"], ["Please provide valid email."]), |
There was a problem hiding this comment.
typo? Also, I don't think we need this check. The user can put pretty much anything they want for username.
We may want to check if an email is valid
it is better to use non-identifying information for the tests, no actual emails and names. We often use names in the tests that convey meaning about the test, for example, "bad_email" (no @) "[email protected]" for example.
Closes #30
Closes #31