Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore bots signoff check #59

Merged
merged 1 commit into from
Aug 14, 2022

Conversation

dolby360
Copy link
Contributor

in case a branch contains bot commits then these commits will not be checked.
this code assumes that the email address of all bots contains [bot].

Signed-off-by: dolby360 [email protected]

Pull Request

Description

Describe what you did and why.

Related issue (if any): fixes #47

Checklist

  • [V] I have followed this repository's contributing guidelines.
  • [V] I have used the conventional commits specification for my commit messages.
  • [V] I have signed my commits.
  • [V] I will adhere to the project's code of conduct.

Additional information

Anything else?

@dolby360 dolby360 requested a review from TomerFi as a code owner August 12, 2022 15:29
@pull-request-size pull-request-size bot added the size: m Pull request has 30 to 100 lines label Aug 12, 2022
@trafico-bot trafico-bot bot added the status: needs review Pull request needs a review label Aug 12, 2022
@TomerFi TomerFi linked an issue Aug 12, 2022 that may be closed by this pull request
Copy link
Owner

@TomerFi TomerFi left a comment

Choose a reason for hiding this comment

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

@dolby360
We have 3 typos failing test, and the CI workflows:


here:

.withArgs(fakeUnknownEmail, sinon.match.func)

change fakeUnknownEmail to fakeBotEmail.


here:

listCommitsStub.resolves({data: [commitSignedByUnknown]});

change commitSignedByUnknown to commitUnsignedBot.


the commit message type is feat!, amend it to feat.

@trafico-bot trafico-bot bot added status: changes requested Pull request changes requested and removed status: needs review Pull request needs a review labels Aug 12, 2022
in case that a branch contains bot commits then these commits will not be checked.
this code assume that the email address of all bots contains [bot].

Signed-off-by: dolby360 <[email protected]>
@dolby360 dolby360 force-pushed the dolev/ignore_bots_signoff_check branch from ca844ee to f780005 Compare August 13, 2022 18:46
@trafico-bot trafico-bot bot added status: needs review Pull request needs a review and removed status: changes requested Pull request changes requested labels Aug 13, 2022
@dolby360 dolby360 changed the title feat!: ignore bots signoff check Ignore bots signoff check Aug 13, 2022
@TomerFi
Copy link
Owner

TomerFi commented Aug 13, 2022

@dolby360
Thank you very much, I will be merging this by tomorrow.

I just want to take the opportunity of this PR and fix the CI issue,
the "Report test results" step fails because of missing permission for forks,
and I can verify a fix with your PR open, so I want to play with it a bit if you don't mind.

Can you please verify the Allow edits by maintainers. checkbox is ticked so I can push a fix here?

@dolby360
Copy link
Contributor Author

Can you please verify the Allow edits by maintainers. checkbox is ticked so I can push a fix here?

Already on. Please take your time there's no rush.

@TomerFi
Copy link
Owner

TomerFi commented Aug 14, 2022

Already on. Please take your time there's no rush.

Weird... I try pushing yesterday, and I got a permissions error, I figured it's the maintainers' thing, but I didn't look anymore into it.

I am looking into it now.

EDIT:
It was some lfs issue on my end, pushed successfully.
thanks!

EDIT 2:
In regards to the failed CI, turns out it's a known issue and it looks like it's going to be resolved soon.
dorny/test-reporter#168

@TomerFi TomerFi force-pushed the dolev/ignore_bots_signoff_check branch 3 times, most recently from c201c70 to f780005 Compare August 14, 2022 13:32
@TomerFi TomerFi merged commit 6d299fa into TomerFi:main Aug 14, 2022
@trafico-bot trafico-bot bot added status: merged Pull request merged and removed status: needs review Pull request needs a review labels Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request has 30 to 100 lines status: merged Pull request merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in identifying bots with the pr-signed-commits handler
2 participants