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

Option to fail on empty description #140

Open
litscher opened this issue Sep 9, 2024 · 3 comments · Fixed by #141
Open

Option to fail on empty description #140

litscher opened this issue Sep 9, 2024 · 3 comments · Fixed by #141
Milestone

Comments

@litscher
Copy link
Contributor

litscher commented Sep 9, 2024

Currently a warning is thrown, but that does not prevent people from merging, when using this action as a branch protection.

@JJ
Copy link
Owner

JJ commented Sep 9, 2024

Reopening until checked and released, to pin tests to this issue.

@JJ JJ reopened this Sep 9, 2024
JJ added a commit that referenced this issue Sep 9, 2024
@JJ
Copy link
Owner

JJ commented Sep 9, 2024

I'm seeing some issue with the logic here. bodyContains already implies allowEmpty == false, so the flow will need to be slightly changed, and maybe a bit of documentation added. Not totally sure that needs to pre-empt the release, though.
If you need this urgently I can make a pre-release as is, changing the minor only.

JJ added a commit that referenced this issue Sep 9, 2024
JJ added a commit that referenced this issue Sep 9, 2024
JJ added a commit that referenced this issue Sep 9, 2024
JJ added a commit that referenced this issue Sep 9, 2024
JJ added a commit that referenced this issue Sep 9, 2024
JJ added a commit that referenced this issue Sep 9, 2024
JJ added a commit that referenced this issue Sep 9, 2024
@JJ
Copy link
Owner

JJ commented Sep 9, 2024

There is definitely a documentation or maybe a default value issue. The default value was included to make it have the same behavior as before, so it's behaving exactly the same, implying this error came from before, namely, bodyContains does not work if the body is empty. I think that the best way to go forward is opening this as a separate issue...

@JJ JJ added this to the v15 milestone Sep 9, 2024
JJ added a commit that referenced this issue Sep 9, 2024
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 a pull request may close this issue.

2 participants