Broken getSha and repo.write

[gmaclennan]

This is currently broken by all the merged pull requests from 11 days ago.

I think it is a problem with the Accept header being set to 'application/vnd.github.v3.raw+json' in #105 but the improved Sha functions etc. from #144 rely on the Accept header being 'application/json' in @darvin commit, but should probably be 'application/vnd.github.v3+json'

However, this change would mean that responses to repo.read with return json with the content base64 encoded, and would break the test on repo.read.

I'm a little confused about how to fix it, since this seems to be a result of different pull requests taking different approaches which are incompatible.

There should also be a test for basic functionality such as writing or updating a file - I'm surprised no one has noticed this breaking change with the new version.

Sorry I can't write a test and help more. It's taken me about 3 hours just trying to understand what is going on and why things are broken now.

[ingalls]

Hey mind giving me a bit more info on the broken repo.write. I'm not currently able to reporduce this.
I added this test today

[gmaclennan]

Try running repo.write twice (first to create the file, then to modify it) and it will fail.
Also, run repo.getSha on any file and it will fail.
Sorry I'm don't really understand tape but I think it's something like this:

    t.test('repo.getSha', function(q) {
        repo.getSha('master', 'README.md', function(err, sha) {
            console.log(err, sha);
            q.ok(res.indexOf('##Setup') !== -1, true, 'Returned REAMDE');
            q.end();
        });
    });

    repo.write('master', 'TEST.md', 'THIS IS A TEST first', 'Creating test', function(err) {
        repo.write('master', 'TEST.md', 'THIS IS A TEST modifying', 'Creating test', function(err) {
            q.error(err);
            q.end();
        });
    });

It breaks on an uncaught error in XMLHttpRequest() because it is expecting a json response but because the accept header is being sent it is getting back a buffer (raw). Other functions though expect back a buffer - it's a problem because with the merged PRs you have functions that were written expecting different types of responses.

[aendra-rininsland]

👍 I'm also experiencing this.

Edit: After a bit of investigating, it all goes pete tong at commit af1e5b5.

[aendra-rininsland]

Looked at this very briefly today. Commenting out line 62 fixes the issue, but it breaks repo.read as per test.repo.js test 6.

Will take a closer look tomorrow, submitting a PR if I find a fix.

[ghing]

I'm seeing this issue as well. Would love to see a fix in master.

[s-a]

Same problem here... Is this repo still supported? Tickets here are very old.

[aendra-rininsland]

@s-a I was going to do a PR on this ages ago but haven't gotten around to it. I'm seriously needing this functionality ASAP, so I'll probably try to do so tomorrow. But yeah, it seems fairly under-maintained given how breaking this issue is...

[aendra-rininsland]

@s-a Oh wow, I've already even done a PR and just forgot about it. Nevermind then...

@michael Are you maintaining this project anymore? If so, would it help if I offered to help co-maintain? My PR from early January that fixes this issue hasn't been merged, and there are ~32 other open pull requests. I can imagine you're super busy with Substance Composer (which looks amazing btw), but I'd really like to see github.js progress — please let me know if there's any way I can help!

[ingalls]

Unfortunately I can't add any contributors but if you want to take over adding tests/testing PRs, I would be happy to merge and roll an npm version for anything you give a 👍.

[aendra-rininsland]

@ingalls Sounds like a plan — will see how far I get on my projects today and will start going through the PRs.

[michael]

@Aendrew I added you as a collaborator. Thanks for the help! :)

[michael]

Only wish I'd have: Try to keep the lib minimal with respect to features. Lib was not intended to be a wrapper around the full Github API, but easy to customize if you need more.

[aendra-rininsland]

@michael Amazing, thank you very much! Will definitely respect that. 😄