-
Notifications
You must be signed in to change notification settings - Fork 795
Add listForks #216
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
Add listForks #216
Conversation
|
Hi. Would you mind add a couple of lines in the README to document this method? Also, can you write a test for it? |
|
I added some documentation to the README, but I don't understand your testing so I'd rather leave that to you. |
|
@mushishi78 Just out of curiosity, what test framework would be more familiar to you? We're rewriting all of our tests using something like Mocha, Jasmine or Qunit; any feedback is quite welcome! |
|
I'm more familiar with Mocha, but I also just didn't understand the tests. For instance, I looked at the test for Now I would have thought the if you listed the branches for this repo (michael/github) then you'd get 'master' and 'gh-pages' but you don't seem to be testing for that. Plus, if I tested for all the forks of this repo, there would be like 349 different forks. I could then test if it got the first few, but that may change in the future with different people deciding to fork or remove their fork of the repo. So you'd need a test repo that'd been forked a few times, but most people wouldn't likely fork on their on volition. |
|
@mushishi78 Ah, okay. Thanks, that's really helpful! Currently, the calls to things like I'm rewriting all the unit tests in #248, any feedback is very much welcome! 😄 |
|
Closed in error; reopening. |
e746e5d to
6f04f13
Compare
|
I've added a basic test for this PR. Merging. |
No description provided.