Uh oh!
There was an error while loading. Please reload this page.
fix(repository): prevents lib from crashing when not providing option… - #588
Conversation
| }); | ||
| it('should successfully write to repo when not providing optional options argument', function(done) { | ||
| remoteRepo.writeFile('master', fileName, initialText, initialMessage, undefined, assertSuccessful(done, function() { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
j-rewerts
commented
Oct 8, 2019
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. |
j-rewerts
commented
Oct 8, 2019
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. |
hazmah0
commented
Oct 10, 2019
No worries about the contribution count. I'll send an email later today, thanks! |
j-rewerts
commented
Oct 30, 2019
LGTM! |
hazmah0
commented
Nov 4, 2019
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! |
…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.