Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

avoid CI typos #26

Closed
Trott opened this issue Mar 22, 2018 · 6 comments
Closed

avoid CI typos #26

Trott opened this issue Mar 22, 2018 · 6 comments

Comments

@Trott
Copy link
Member

Trott commented Mar 22, 2018

So, this happened: nodejs/node#19438 (comment)

(Someone accidentally used the wrong PR number when starting a CI job.)

Surely this isn't the first time this has happened nor the last time this will happen.

Would be awesome if we could either:

  • have git node land check the CI parameters to make sure they match the PR number? (That's a big request and would probably be sloooooow.)

  • have a tool to start CI jobs that would somehow reduce the probability of this sort of thing. Not sure what that would look like to be honest.

This may be Too Big To Solve. Feel free to close. Just wanted to put it out there for consideration. Thanks.

@vsemozhetbyt
Copy link

vsemozhetbyt commented Mar 24, 2018

As an awkward workaround till this is done properly, I can suggest a tiny bookmarklet with a new Google Chrome clipboard API (available since v66):

javascript: {
  (async () => {
    try {
      const PR_ID = location.pathname.replace(/.+\/pull\/(\d+).*/, '$1');

      if (confirm(`Create CI for #${PR_ID} ("${document.title}")?`)) {
        await navigator.clipboard.writeText(PR_ID);
        open('https://ci.nodejs.org/job/node-test-pull-request/build?delay=0sec');
      }
    } catch (err) {
      alert(err);
    }
  })();
}

It asks if the PR is an intended one, copies its ID to the clipboard and opens CI form.

@priyank-p
Copy link

The way @vsemozhetbyt suggested could be easily done in node-review and it's a perfect place for it.

@richardlau
Copy link
Member

When the github bot was able to start citgm CI runs on request, it worked by collaborators posting a comment in the PR @-mentioning the bot which would then work out the correct parameters. It worked really well when it worked and would be a good long term solution.

@joyeecheung
Copy link
Member

joyeecheung commented Mar 26, 2018

have git node land check the CI parameters to make sure they match the PR number? (That's a big request and would probably be sloooooow.)

Not really, actually, unless the Jenkins is stucked for some reason. All you need is an additional request to the Jenkins API. In case anyone is interested, here is how you can figure out the PRID from a build (code copy-pasted from nodejs/node-core-utils#161, and from my experience using that PR to get the CI results (~3-5 req per PR failed by flakes) , the request is pretty fast if Jenkins is not stucked):

// request a build with the suffix api/json/?tree=actions[parameters[name,value]]
// e.g. https://ci.nodejs.org/job/node-test-pull-request/13871/api/json?tree=actions[parameters[name,value]]
// https://ci.nodejs.org/job/node-test-commit/17170/api/json?tree=actions[parameters[name,value]]

  get sourceURL() {
    const { params } = this;

    if (params.PR_ID) {  // from a node-test-pull-request build
      const owner = params.TARGET_GITHUB_ORG;
      const repo = params.TARGET_REPO_NAME;
      const prid = params.PR_ID;
      return `https://github.com/${owner}/${repo}/pull/${prid}/`;
    }

    if (params.GITHUB_ORG) {  // from a node-test-commit build
      const owner = params.GITHUB_ORG;
      const repo = params.REPO_NAME;
      const prm = params.GIT_REMOTE_REF.match(/refs\/pull\/(\d+)\/head/);
      if (prm) {
        return `https://github.com/${owner}/${repo}/pull/${prm[1]}/`;
      } else {
        const result =
          `https://api.github.com/repos/${owner}/${repo}/git/` +
          params.GIT_REMOTE_REF;
        return result;
      }
    }
  }

@gibfahn
Copy link
Member

gibfahn commented Mar 26, 2018

When the github bot was able to start citgm CI runs on request, it worked by collaborators posting a comment in the PR @-mentioning the bot which would then work out the correct parameters. It worked really well when it worked and would be a good long term solution.

Most recently discussed in nodejs/build#1180, agreed that this is the best solution long-term.

I like the idea of git node metadata warning if the PR-URL doesn't match, given that it's already telling you whether CI was run or not.

@Trott
Copy link
Member Author

Trott commented Apr 22, 2023

Unarchived this repo so I could close all the PRs and issues. Will re-archive when I'm done.

@Trott Trott closed this as completed Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants