-
Notifications
You must be signed in to change notification settings - Fork 47
Update community-benchmark/run.sh to allow generic github repos/branches #195
Conversation
Ping @nodejs/build re ^^^ It would be nice to have this + #194 landed at some point, they should be ready as far as the PRs are concerned. If I can do anything to make this happen, I’ll be happy to. |
@gareth-ellis I don't quite understand what you are asking @gibfahn to do with respect to the job? Is it that you want a new job to be able to test? If so I created this copy for you https://ci.nodejs.org/job/benchmark-node-micro-benchmarks-test/. If you need anything more to get #194 and this one please let me know. |
The change @addaleax is to add the ability to pull a branch from another repo into the 2nd usecase of the core benchmarks script. Currently the jenkins job only caters for the 1st usecase, so a second job needed creating. As @addaleax had started a thread in build regarding this, i thought asking for it to be done here made sense. I will land this shortly along with #194 |
@gareth-ellis ok, you should be able to change the name of the new job to something appropriate. |
AIUI there shouldn't be a need for a second job, we can just add the two new args to the first job with appropriate defaults. |
@addaleax this looks to be waiting on a rebase from you. |
Ping @addaleax |
@BridgeAR There’s not much more to do here for me, this needs support from somebody from @nodejs/build. And it looks like they are abandoning this idea anyway (I don’t know why, though). |
(iirc the code here is still ready, the Jenkins interface just never worked and I can’t access/modify that.) |
@addaleax I'm happy to land this but I don't run the job myself so would like somebody to test is out to make sure we've not broken existing usage. Once we do that we can add the jenkins config and then repeat. |
@nodejs/build Is this something that we can/should accommodate in Jenkins or should this be closed? |
@Trott are you asking because you want to use it or just because the issue has been open a long time? |
Because the issue has been open a long time. Just triaging. If the right answer is "No one knows yet, so let's leave it open for a while longer", that's fine. |
@Trott I'm looking for a volunteer to help validate if I land it. Given that we've not had a volunteer for a long time I think we could close and re-open if somebody shows an interest. |
Benchmarking WG has been de-chartered and going to archive this repo. Please open issue in build repo if this is still needed. |
Refs: nodejs/build#1059
/cc @gareth-ellis @gibfahn
@gibfahn I think after this all that would need to be done to fix nodejs/build#1059 is to expose a second CI job that takes the arguments defined under “Use case 2” here
And in any case it would be really nice to expose the
FILTER
variable in the UI as well