4 ms·

"test every commit before allowing merge" I just set this up yesterday for a new codebase with GitHub actions. Very straightforward, took an hour or two. Onl

"test every commit before allowing merge" I just set this up yesterday for a new codebase with GitHub actions. Very straightforward, took an hour or two. Only odd part was that the GitHub actions wizard saves the yaml in the root instead of .github/workflows where it needs to be.

Liskni_si 6y ago
Would you mind sharing how to do this, please?
- davidjnelson 6y ago
  Sure np. Directions vary depending on what you’re doing, but for a React app this will work. So click on the actions tab in the repo, and click the nodejs template. Change the run command to the npm script you want to run like npm run ci-check. It will do something along the lines of npm ci && npm run lint-check && npm run format-check && npm run test && npm run build. Save that. Then move it from the repo root into a new subdirectory you’ll need to create called .github/workflows. If you are an admin, click the settings tab and protect your merge branch so you can only merge if this succeeds. That’s it!
  - Liskni_si 6y ago
    Oh, that's a misunderstanding then. I think what was meant by "test every commit before allowing merge" was testing each individual commit in a PR branch, not just testing PRs before merging. I don't think your solution does this. :-(
    - davidjnelson 6y ago
      This setup does indeed test every commit in a pr branch. For the “on” property, it only has to be set to “pull_request” to do so. If you also add “commit”, it runs it twice. It does exactly what you wanted it to. Feel free to email me if you need help setting it up, happy to help. Cheers!
      - Liskni_si 6y ago
        This is really weird. The nodejs template (https://github.com/actions/starter-workflows/blob/master/ci/node.js.yml https://github.com/actions/starter-workflows/blob/master/ci/...) is set up to test on "pull_request" and "push". There's no mention of "commit" anywhere in the documentation, and when I try to add it there in the GitHub editor, it tells me it's incorrect. Are we talking about the same thing?
        davidjnelson 6y ago
        Setting “on” to pull_request runs my action for each commit I push to a PR branch.
        Liskni_si 6y ago
        But this only works if you push each commit separately. If you push them all at once, it only tests the last one. Also, whenever you rebase, only the last commit is tested. I tested this here: https://github.com/liskin/nodejs-hello-world/pull/1 https://github.com/liskin/nodejs-hello-world/pull/1

Liskni_si 6y ago

Would you mind sharing how to do this, please?

davidjnelson 6y ago
Sure np. Directions vary depending on what you’re doing, but for a React app this will work. So click on the actions tab in the repo, and click the nodejs template. Change the run command to the npm script you want to run like npm run ci-check. It will do something along the lines of npm ci && npm run lint-check && npm run format-check && npm run test && npm run build. Save that. Then move it from the repo root into a new subdirectory you’ll need to create called .github/workflows. If you are an admin, click the settings tab and protect your merge branch so you can only merge if this succeeds. That’s it!
- Liskni_si 6y ago
  Oh, that's a misunderstanding then. I think what was meant by "test every commit before allowing merge" was testing each individual commit in a PR branch, not just testing PRs before merging. I don't think your solution does this. :-(
  - davidjnelson 6y ago
    This setup does indeed test every commit in a pr branch. For the “on” property, it only has to be set to “pull_request” to do so. If you also add “commit”, it runs it twice. It does exactly what you wanted it to. Feel free to email me if you need help setting it up, happy to help. Cheers!
    - Liskni_si 6y ago
      This is really weird. The nodejs template (https://github.com/actions/starter-workflows/blob/master/ci/node.js.yml https://github.com/actions/starter-workflows/blob/master/ci/...) is set up to test on "pull_request" and "push". There's no mention of "commit" anywhere in the documentation, and when I try to add it there in the GitHub editor, it tells me it's incorrect. Are we talking about the same thing?
      - davidjnelson 6y ago
        Setting “on” to pull_request runs my action for each commit I push to a PR branch.
        Liskni_si 6y ago
        But this only works if you push each commit separately. If you push them all at once, it only tests the last one. Also, whenever you rebase, only the last commit is tested. I tested this here: https://github.com/liskin/nodejs-hello-world/pull/1 https://github.com/liskin/nodejs-hello-world/pull/1