Skip to content

travis: add nan, llnode and node-core-utils #156

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

Merged
merged 2 commits into from
Nov 16, 2017

Conversation

joyeecheung
Copy link
Member

This enables travis intergrations to repos that need it that I am aware of.

cc @phillipj

@joyeecheung
Copy link
Member Author

I don't think we have the equivalent for appveyor here, nan and node-core-utils would need that as well. I can try to implement one.

@phillipj
Copy link
Member

@Fishrock123 are you willing to temporarily open up for Travis and adding the necessary webhooks to these repos? If so, adding the github-bot would also be very nice!

@joyeecheung
Copy link
Member Author

Added core-validate-commit as well.

@phillipj I can add the webhooks but I cannot get the Travis token under the nodejs account..anyone knows how to get those?

@phillipj
Copy link
Member

@joyeecheung hm, I wondering if Travis has been connected to @Fishrock123's github.com account?

Me + some others in the @nodejs/build group can get a hold of the secret needed to setup the webhook from github.com -> github-bot when needed.

@Fishrock123
Copy link
Contributor

It probably has been yeah

@Fishrock123
Copy link
Contributor

Idk anything about travis really so... any ideas?

@Fishrock123
Copy link
Contributor

To be honest, I don't think I can even check without enabling access to the org, is that clear to do now?

@rvagg
Copy link
Member

rvagg commented Nov 13, 2017

sorry @phillipj, @Fishrock123 and I are poking around at travis and can't figure out how this is all linked up, it's been too long since I looked at this and @Fishrock123 doesn't remember either, got any hints on what we need to do here?

@rvagg
Copy link
Member

rvagg commented Nov 13, 2017

the travis token that the bot has isn't mine or @Fishrock123's, we've just confirmed that

@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 14, 2017

EDIT: nope, turns out you don't event have to configure the username and the token, you just need the hook to be active, so ignore this comment and see #156 (comment)

I think to enable it we just need to configure this webhook, which can be added by searching travis on the "Integration & service" setting.

screen shot 2017-11-14 at 9 07 08 am

The "user" should be "nodejs" and the token should be on the profile page of the nodejs travis account, similar to this one

screen shot 2017-11-14 at 9 08 35 am

the travis token that the bot has isn't mine or @Fishrock123's, we've just confirmed that

@rvagg I think the token the bot has is the token under the Node.js account? https://travis-ci.org/nodejs If so we just need to use the token to configure webhooks for these repos and we are good to go?

@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 14, 2017

Also, somehow node-core-utils and llnode have been able to send PR and push event to Travis without configuring the service hooks (maybe because they are transferred into the organization), but I guess reconfiguring the hooks wouldn't hurt.

@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 14, 2017

Hmmm..I looked into the setting of the nodejs.org repo and I think it actually works like this:

  1. Github sends PR/Push events to both Travis and the github bot's webhook. Somehow it doesn't have to send events to the nodejs Travis account, for example, the nodejs.org repo is linked to Fishrock123 's account, llnode is linked to indutny's account, and node-core-utils isn't even linked to anyone's account. As long as there is that hook and you've got travis.yml in your repo, and the travis page of that repo (e.g. https://travis-ci.org/nodejs/node-core-utils) seem to be working, the Travis end is hooked up with the right owner and the right repo.
  2. The bot would start to poll PR status from Travis after receiving the event, and update the build status.

I just checked out the Travis pages:

They all seem to be working, it's only the github bot webhook that has not been configured.

So my guess is, if we merge this PR, and configure the github bot hook for each repo, the bot will start to update the build status for these repo. To configure the github bot hooks, all we need is GITHUB_WEBHOOK_SECRET described in the github bot's README.

@phillipj
Copy link
Member

Totally fine by me to add the necessary github-bot webhook to see if that's enough.

The GITHUB_WEBHOOK_SECRET secret is in our secrets repo. Ping me on IRC or someone else in the Build WG's infra group to get a hold of it.

@joyeecheung
Copy link
Member Author

@phillipj @rvagg I have tried to configure a webhook for node-core-utils and it seem to be working:

Connection: keep-alive
Content-Length: 0
Date: Thu, 16 Nov 2017 05:02:55 GMT
X-Powered-By: Express
X-Request-Id: a50ea40d-5e8c-44c3-aeb7-aa94a5b5d202

I would need to merge this request so the bot would actually start to poll the build status instead of ignoring it. Can I merge this now?

@joyeecheung
Copy link
Member Author

Also, will the bot redeploy itself automatically after this has been merged?

phillipj

This comment was marked as off-topic.

@rvagg
Copy link
Member

rvagg commented Nov 16, 2017

merges to master should trigger a redeploy of the bot on its server

@phillipj
Copy link
Member

phillipj commented Nov 16, 2017

Yepp, that's correct.

That flow works like this:

  1. Webhook in this repo makes github.com sends a request to the github-bot server
  2. Webhook request is received by github-webhook with a custom github-bot .json-config
  3. If commit(s) landed in master, the redeploy script is executed: https://github.com/nodejs/build/blob/master/setup/github-bot/resources/deploy-github-bot.sh

@joyeecheung joyeecheung merged commit af20ad8 into nodejs:master Nov 16, 2017
@joyeecheung
Copy link
Member Author

@rvagg @phillipj I went ahead and merged this PR, let's see if it works properly

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.

4 participants