Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upfix(repository): prevents lib from crashing when not providing option… #588
Conversation
…al arguments
| @@ -385,6 +385,17 @@ describe('Repository', function() { | |||
| })); | |||
| }); | |||
|
|
|||
| it('should successfully write to repo when not providing optional options argument', function(done) { | |||
| remoteRepo.writeFile('master', fileName, initialText, initialMessage, undefined, assertSuccessful(done, function() { | |||
This comment has been minimized.
This comment has been minimized.
j-rewerts
Oct 8, 2019
Member
I think the reason people are running into this bug is because they're using Promises. The 2 option-less calls I'm imagining people are making:
- Using callbacks:
remoteRepo.writeFile('master', fileName, initialText, initialMessage, assertSuccessFunction)- Using Promises:
remoteRepo.writeFile('master', fileName, initialText, initialMessage).then(res => {})I'd like tests to verify the Promise approach, as we have many testing writing files with no options provided, but with a callback.
This comment has been minimized.
This comment has been minimized.
hazmah0
Oct 8, 2019
Author
Contributor
True. Thinking back, I stumbled upon this using async/await. If one were to use the callback approach this would work fine since there is a check in the beginning to see if options is a function and in that case use that as the callback.
|
Apologies for the delay @hazmah0. I've been traveling. Please make the requested changes. As for the testing account, feel free to email me and I can get you access, or alternatively, feel free to use one of your own GitHub accounts. Just note that running the test suite will create and edit a bunch of things, so it's best to avoid using your primary account. |
|
Also, I'm not too sure how Hacktoberfest counts contributions, but if you need to close and reopen this to get it to count towards your 5, feel free to do that. |
|
No worries about the contribution count. I'll send an email later today, thanks! |
|
LGTM! |
|
Just noticed that the CI is failing the lint task due to a missing semicolon at line 395. I can submit a new PR with that fix later today! |
hazmah0 commentedSep 19, 2019
…al arguments
Ran into this issue today described in this old PR: #456
This provides the same fix and adds a test case, though I can't run the tests since it asks me to email jaredrewerts@gmail.com to get access.