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

Fix new versions not to use WeakCallbackInfo::IsFirstPass #569

Merged
merged 1 commit into from
May 28, 2016

Conversation

kkoopa
Copy link
Collaborator

@kkoopa kkoopa commented May 10, 2016

Follow-up to #568.

@bnoordhuis
Copy link
Member

LGTM, I think.

@kkoopa
Copy link
Collaborator Author

kkoopa commented May 25, 2016

Good. Thanks for looking through it. This was the least invasive solution I could come up with. It should even have better performance than the old implementation, thanks to dead code elimination. CI also passes, but in somehow the hook part showing CI status in PRs has disappeared in the last months.
I'll merge this and the related PR this week and publish a new patch version.

@rvagg Could you figure out what's what with the Travis and AppVeyor hooks for PRs?

@kkoopa kkoopa merged commit 615c19d into nodejs:master May 28, 2016
@kkoopa kkoopa deleted the weakness branch May 28, 2016 15:28
@rvagg
Copy link
Member

rvagg commented May 31, 2016

Unfortunately the nodejs org doesn't allow third-party applications, it's been a policy for a while but tbh I don't recall how it started or exactly what the justification is. That's why Travis and AppVeyor can't post status back via the GH API.

@kkoopa
Copy link
Collaborator Author

kkoopa commented May 31, 2016

Aha, well that explains why it stopped working. Cannot access be granted for specific repositories? The current state is quite error prone, as it is easy to forget checking builds manually.

@bnoordhuis
Copy link
Member

It depends on the permissions that the third-party application wants. In the case of Travis CI, it wanted (and presumably still wants) permissions that lets it access private repositories. Because our private repositories contain sensitive information, and because we can't trust a third-party not to get hacked, that makes it a no-go.

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.

3 participants