Skip to content

Conversation

@kaworu
Copy link
Member

@kaworu kaworu commented Aug 28, 2023

Add regexpMsg to require-msgs-in-commit, a regular expression alternative to msg. Also brings client mode support for require-msgs-in-commit on the way so I could manually test the patch.

client mode tests

Got a complain when checking for a missing msg:

% cat check-msg-for-co-authored-by.yaml
require-msgs-in-commit:
  - msg: "Co-authored-by"
    helper: "https://docs.cilium.io/en/stable/contributing/contributing/#developer-s-certificate-of-origin"
    set-labels:
      - "dont-merge/needs-sign-off"
% ./github-actions -client-mode -org kaworu -repo cilium -branch master -pr 287 -config check-msg-for-co-authored-by.yaml

No complain on kaworu/cilium#283 for an existing msg:

% cat check-msg-for-signed-off-by.yaml
require-msgs-in-commit:
  - msg: "Signed-off-by"
    helper: "https://docs.cilium.io/en/stable/contributing/contributing/#developer-s-certificate-of-origin"
    set-labels:
      - "dont-merge/needs-sign-off"
% ./github-actions -client-mode -org kaworu -repo cilium -branch master -pr 283 -config check-msg-for-signed-off-by.yaml

Got an error when neither msg or regexpMsg is configured:

% cat check-none.yaml
require-msgs-in-commit:
  - helper: "https://docs.cilium.io/en/stable/contributing/contributing/#developer-s-certificate-of-origin"
    set-labels:
      - "dont-merge/needs-sign-off"
% ./github-actions -client-mode -org kaworu -repo cilium -branch master -pr 281 -config check-none.yaml
Getting PRs from GH
panic: no msg or regexpMsg configured

goroutine 1 [running]:
main.runClient()
	/home/alex/go/src/github.com/cilium/github-actions/cmd/client.go:106 +0x748
main.main()
	/home/alex/go/src/github.com/cilium/github-actions/cmd/main.go:34 +0x18

Complains about no regexpMsg match:

% cat check-regexpmsg-for-co-authored-by.yaml
require-msgs-in-commit:
  - regexpMsg: "(?m)^Co-authored-by:"
    helper: "https://docs.cilium.io/en/stable/contributing/contributing/#developer-s-certificate-of-origin"
    set-labels:
      - "dont-merge/needs-sign-off"
% ./github-actions -client-mode -org kaworu -repo cilium -branch master -pr 280 -config check-regexpmsg-for-co-authored-by.yaml

No complain on kaworu/cilium#277 when regexpMsg matches:

% cat check-regexpmsg-for-signed-off-by.yaml
require-msgs-in-commit:
  - regexpMsg: "(?m)^Signed-off-by:"
    helper: "https://docs.cilium.io/en/stable/contributing/contributing/#developer-s-certificate-of-origin"
    set-labels:
      - "dont-merge/needs-sign-off"
% ./github-actions -client-mode -org kaworu -repo cilium -branch master -pr 277 -config check-regexpmsg-for-signed-off-by.yaml

kaworu added 5 commits August 28, 2023 10:16
Signed-off-by: Alexandre Perrin <[email protected]>
Before this patch when running `make clean` we get:

    make: *** No rule to make target 'rm', needed by 'clean'.  Stop.

Signed-off-by: Alexandre Perrin <[email protected]>
Comment on lines -172 to +177
var cfg github.FlakeConfig
var cfg github.PRBlockerConfig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aanm is this breaking the current client mode workflow? I guess the FlakeConfig has now to be under flake-tracker:, is that a problem?

Copy link
Member

@aanm aanm Aug 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine, client mode is meant for development purposes anyway.

Comment on lines -172 to +177
var cfg github.FlakeConfig
var cfg github.PRBlockerConfig
Copy link
Member

@aanm aanm Aug 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine, client mode is meant for development purposes anyway.

@aanm aanm merged commit 85af3d5 into cilium:master Aug 28, 2023
@kaworu kaworu deleted the pr/kaworu/regexp-for-require-msgs-in-commit branch August 28, 2023 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants