-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
Automatic CI for pull requests (progress tracking issue) #1415
Comments
I'd really rather not have a bot. they are usually very noisy. Can we hook into GitHub's built-in Ci status for PRs? |
hmm, that might be do-able. EDIT: below would be RAD |
@rvagg I think the GH api support multiple "checks" per ci run. How about using one for each slave group (ui example here - click green checkmark)? The downside of top posting a status board is that it'd probably have to contain what revision it last tested. Github sunsets previous test runs, comments, etcetera when new commits are added/force pushed and the "overall" ci/merge status is always at the bottom. |
the top-posted badge only needs to be inserted once and the badge itself can contain details about the run, it's not a static image. but yes, the multiple status checks is an interesting feature I wasn't aware of! and you even get individual links, let's start with that then because it's probably going to be easier anyway. |
@rvagg I was thinking of using it for other steps too; linting, commit message/author checks, etc. The "proxy" would be responsible to delegate different tasks to different workers. |
👍 sounds like a plan |
from the github docs:
so there'll be a hard-limit of 1000 on the number of build slave groups we can have for io.js, good to know. |
That almost sounds like the combination of how many checks we make and how many times we make them on a PR must be less than 1000.. (Not that that will be an issue really) |
The "context" is the label used for a commit status, and updating a status with a context that has already been set overwrites the previous value.. I'm pretty sure if a PR had that many statuses set on its head commit there would be complaints from devs looking at it long before we hit 1000. Cool to know the limit is high enough to allow for other kids of automation/workflow, though. |
@rvagg Could it be made partial? Lint run is fast, for example. Even having just lint checks as status badges would be good. |
Is this still something that's wanted? If so, is there anything actionable at this time? Maybe some progress has happened that wasn't noted here? |
@Trott a lot of partial progress has been made, but for now the conclusion is pretty much that we shouldn't proceed with automatic testing. I have other ideas/plans for linting and perhaps even commit message validation though! |
A few things is starting to brew over here: https://github.com/nodejs-github-bot/github-bot/issues |
@jbergstroem Any objection to closing this issue? |
/cc @iojs/build
This is not done yet but I've spent some quality time with my head in code (as a leisure activity!) and have made good enough progress at this point to start turning it on and experimenting with it.
There are 3 components I'm working on:
I've hooked up what I have to this repo and re-delivered a webhook notification for the last PR #1412 and it's triggered a build here: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/497/
The containers build jobs need a lot of love because they just haven't been used and we have problems with git, file descriptors and some other random things that keep them from being useful.
Not sure what the timeline is for getting all of this working but one of my next tasks is to get the code of this up in a repo on iojs so others can contribute. I'll post status updates here but at least for now at least collaborators can trigger CI runs themselves without having full Jenkins access.
The text was updated successfully, but these errors were encountered: