Adding a job that enforce a single commit for each PR. #20

ybettan · 2022-08-30T17:49:46Z

Adding a job that enforce a single commit for each PR.

This job will make sure that all commits in a PR were squashed to a single
commit.

Signed-off-by: Yoni Bettan [email protected]

k8s-ci-robot · 2022-08-30T17:49:54Z

Hi @ybettan. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ybettan · 2022-08-30T17:50:47Z

/cc @qbarrand
/cc @yevgeny-shnaidman
/cc @enriquebelarte

k8s-ci-robot · 2022-08-30T17:50:49Z

@ybettan: GitHub didn't allow me to request PR reviews from the following users: enriquebelarte.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @qbarrand
/cc @yevgeny-shnaidman
/cc @enriquebelarte

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yevgeny-shnaidman · 2022-08-31T06:17:39Z

@ybettan why do we need the PR description and commit message to be identical? why is it a hard requirement?

ybettan · 2022-08-31T08:50:14Z

@yevgeny-shnaidman I think that the commit message should be very descriptive and should include all the info required for understanding the code change.

By enforcing identical commit message and PR description, the reviewer of the PR can just look at the PR description without the need of also checking the commit message inside the PR in order to make sure the commit message wasn't neglected.

In addition, you can add additional info such as reviewers or any other text that is not relevant for describing the change by adding --- at the end of the PR description and all the text after that will be ignored by the API and doesn't have to match the commit message.

qbarrand · 2022-08-31T09:18:00Z

/hold
Hold while we compare those requirements with what the Kubernetes contributor cheat sheet recommends.

ybettan · 2022-08-31T10:18:55Z

As agreed, this PR is going to be split into 2 PRs:

One PR to add a job to enforce a single commit per PR
One PR to enforce the sync between the PR description and the commit message

Both PR will start as optional jobs and the second one might be removed after some time if it causes more issues than benefits.

qbarrand · 2022-08-31T10:43:59Z

@ybettan please rebase.

qbarrand · 2022-08-31T10:47:42Z

/ok-to-test

qbarrand · 2022-08-31T10:50:37Z

ci/prow/check-commits-number

+number_of_commits=$(curl -H 'Accept: application/vnd.github+json' \
+    https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/pulls/${PULL_NUMBER} | jq '.commits')
+if [[ ${number_of_commits} != 1 ]]; then


I think we could use git to do that: git rev-list --count HEAD ^main

Good point.

Fixed.

qbarrand · 2022-08-31T12:27:58Z

ci/prow/check-commits-number

@@ -0,0 +1,12 @@
+#!/usr/bin/env bash


I'd rename this to ci/prow/check-commits-count. WDYT?

Bumps [github.com/google/go-cmp](https://github.com/google/go-cmp) from 0.5.5 to 0.5.7. - [Release notes](https://github.com/google/go-cmp/releases) - [Commits](google/go-cmp@v0.5.5...v0.5.7) --- updated-dependencies: - dependency-name: github.com/google/go-cmp dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@‌github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@‌users.noreply.github.com>

This job will make sure that all commits in a PR were squashed to a single commit. Signed-off-by: Yoni Bettan <[email protected]>

qbarrand · 2022-08-31T14:53:38Z

/unhold
/lgtm
/approved

qbarrand · 2022-08-31T14:55:40Z

/approve

k8s-ci-robot · 2022-08-31T14:55:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qbarrand, ybettan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [qbarrand]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 30, 2022

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 30, 2022

ybettan changed the title ~~Adding a job that checks the PR and commit formats.~~ [WIP]: Adding a job that checks the PR and commit formats. Aug 30, 2022

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 30, 2022

k8s-ci-robot requested review from qbarrand and yevgeny-shnaidman August 30, 2022 17:49

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 30, 2022

ybettan force-pushed the check-commit branch 2 times, most recently from cc3dab4 to c50cec1 Compare August 30, 2022 19:28

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 30, 2022

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 31, 2022

ybettan changed the title ~~[WIP]: Adding a job that checks the PR and commit formats.~~ Adding a job that checks the PR and commit formats. Aug 31, 2022

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 31, 2022

ybettan force-pushed the check-commit branch from c50cec1 to c615260 Compare August 31, 2022 10:46

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 31, 2022

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 31, 2022

qbarrand reviewed Aug 31, 2022

View reviewed changes

ybettan changed the title ~~Adding a job that checks the PR and commit formats.~~ Adding a job that enforce a single commit for each PR. Aug 31, 2022

ybettan force-pushed the check-commit branch from c615260 to bcc4b6b Compare August 31, 2022 11:22

ybettan mentioned this pull request Aug 31, 2022

Adding check-commits-number job to KMM. kubernetes/test-infra#27323

Merged

qbarrand reviewed Aug 31, 2022

View reviewed changes

Adding a job that enforce a single commit for each PR.

f43286d

This job will make sure that all commits in a PR were squashed to a single commit. Signed-off-by: Yoni Bettan <[email protected]>

ybettan force-pushed the check-commit branch from bcc4b6b to f43286d Compare August 31, 2022 14:19

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 31, 2022

k8s-ci-robot assigned qbarrand Aug 31, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 31, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 31, 2022

k8s-ci-robot merged commit e0a5b43 into kubernetes-sigs:main Aug 31, 2022

ybettan deleted the check-commit branch September 1, 2022 08:02

Adding a job that enforce a single commit for each PR. #20

Adding a job that enforce a single commit for each PR. #20

Uh oh!

Conversation

ybettan commented Aug 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Aug 30, 2022

Uh oh!

ybettan commented Aug 30, 2022

Uh oh!

k8s-ci-robot commented Aug 30, 2022

Uh oh!

yevgeny-shnaidman commented Aug 31, 2022

Uh oh!

ybettan commented Aug 31, 2022

Uh oh!

qbarrand commented Aug 31, 2022

Uh oh!

ybettan commented Aug 31, 2022

Uh oh!

qbarrand commented Aug 31, 2022

Uh oh!

qbarrand commented Aug 31, 2022

Uh oh!

qbarrand Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ybettan Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

qbarrand Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

ybettan Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

qbarrand commented Aug 31, 2022

Uh oh!

qbarrand commented Aug 31, 2022

Uh oh!

k8s-ci-robot commented Aug 31, 2022

Uh oh!

Uh oh!

ybettan commented Aug 30, 2022 •

edited

Loading

qbarrand Aug 31, 2022 •

edited

Loading