Skip to content
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

Remove github OAuth token #641

Closed
wants to merge 1 commit into from
Closed

Remove github OAuth token #641

wants to merge 1 commit into from

Conversation

danez
Copy link
Contributor

@danez danez commented Oct 11, 2016

Summary

Fixes #612
This does not call github.authenticate() if there is no token provided. Maybe even removing the call at all now that yarn is OS?

This only fixes the authentication issue, after I fixed this it seems yarn is trying to download an rpm file on my mac. Going to look into this next.

info Downloading asset "yarn-0.15.0-1.noarch.rpm" from release "v0.15.0"
error https://api.github.com/repos/yarnpkg/yarn/releases/assets/2456563?access_token=undefined: invalid tar file
    at Extract.Parse._startEntry (/Users/danieltschinder/.yarn/node_modules/tar/lib/parse.js:149:13)
    at Extract.Parse._process (/Users/danieltschinder/.yarn/node_modules/tar/lib/parse.js:131:12)
    at BlockStream.<anonymous> (/Users/danieltschinder/.yarn/node_modules/tar/lib/parse.js:47:8)
    at emitOne (events.js:77:13)
    at BlockStream.emit (events.js:169:7)
    at BlockStream._emitChunk (/Users/danieltschinder/.yarn/node_modules/block-stream/block-stream.js:145:10)
    at BlockStream.flush (/Users/danieltschinder/.yarn/node_modules/block-stream/block-stream.js:70:8)
    at BlockStream.end (/Users/danieltschinder/.yarn/node_modules/block-stream/block-stream.js:66:8)
    at Extract.Parse.end (/Users/danieltschinder/.yarn/node_modules/tar/lib/parse.js:86:23)
    at UnpackStream.onend (_stream_readable.js:498:10)

Test plan

Well, there are tests for self-update but seems they are disabled. I try to disable them in the upcoming PR :)

@sebmck
Copy link
Contributor

sebmck commented Oct 11, 2016

Awesome! Could you remove the token stuff all together please? We wont need it now that we're open source.

@danez
Copy link
Contributor Author

danez commented Oct 11, 2016

I changed it, but for some reason I cannot squash them locally because, I always have changed files in my working copy on a mac and cannot get rid of them (Some CRLF vs. LF) problem it seems Seems a new checkout again fixed it

@danez danez mentioned this pull request Oct 11, 2016
@danez danez changed the title Only do authentication if token provided Remove github OAuth token Oct 11, 2016
@sebmck
Copy link
Contributor

sebmck commented Oct 11, 2016

Thank you! @bestander can you please review since you're most familiar with this piece!

@bestander
Copy link
Member

@danez, could you check if tests are passing if you undisable them?
Also feel free to startup a discussion whether yarn should be evergreen and selfupdate automatically

@danez
Copy link
Contributor Author

danez commented Oct 11, 2016

No, see #676. For now I also did this change there to see that the tests work, we might close this PR here if you want the tests to run for this change.

@danez
Copy link
Contributor Author

danez commented Oct 12, 2016

Okay i close this in favor of #676

@danez danez closed this Oct 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

self-update throws error
3 participants