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

Regex validation for description #69

Closed
denis-ev opened this issue Jan 3, 2021 · 15 comments · Fixed by #71
Closed

Regex validation for description #69

denis-ev opened this issue Jan 3, 2021 · 15 comments · Fixed by #71

Comments

@denis-ev
Copy link

denis-ev commented Jan 3, 2021

Check does not fail when PR Title looks like: feat: Add something
but it should fail, semantic release does not allow it. This is only valid for the first letter after feat:
After that it does not matter if there is a upper-case letter.

Run wagoid/commitlint-github-action@v2
8
/usr/bin/docker run --name wagoidcommitlintgithubaction216_474391 --label cc4956 --workdir /github/workspace --rm -e INPUT_CONFIGFILE -e INPUT_FIRSTPARENT -e INPUT_FAILONWARNINGS -e INPUT_HELPURL -e INPUT_TOKEN -e HOME -e GITHUB_JOB -e GITHUB_REF -e GITHUB_SHA -e GITHUB_REPOSITORY -e GITHUB_REPOSITORY_OWNER -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RETENTION_DAYS -e GITHUB_ACTOR -e GITHUB_WORKFLOW -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e GITHUB_EVENT_NAME -e GITHUB_SERVER_URL -e GITHUB_API_URL -e GITHUB_GRAPHQL_URL -e GITHUB_WORKSPACE -e GITHUB_ACTION -e GITHUB_EVENT_PATH -e GITHUB_ACTION_REPOSITORY -e GITHUB_ACTION_REF -e GITHUB_PATH -e GITHUB_ENV -e RUNNER_OS -e RUNNER_TOOL_CACHE -e RUNNER_TEMP -e RUNNER_WORKSPACE -e ACTIONS_RUNTIME_URL -e ACTIONS_RUNTIME_TOKEN -e ACTIONS_CACHE_URL -e GITHUB_ACTIONS=true -e CI=true -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/_temp/_runner_file_commands":"/github/file_commands" -v "/home/runner/work/VivumLab/VivumLab":"/github/workspace" wagoid/commitlint-github-action:2.1.6
9
::stop-commands::3ee5cac1-f660-4061-86b6-5883ca3b608a
10
::error::You have commit messages with errors%0A%0A⧗   input: chore: Work in Progress action revert (#254)%0A%0ASigned-off-by: Denis Evers <denis-ev@users.noreply.github.com%0A✖   subject must not be sentence-case, start-case, pascal-case, upper-case [subject-case]%0A%0A✖   found 1 problems, 0 warnings%0Aⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint%0A
11
::3ee5cac1-f660-4061-86b6-5883ca3b608a::
@amannn
Copy link
Owner

amannn commented Jan 5, 2021

Hi @denis-ev,

thank you for the report. I think I'd need a bit more information here.

semantic release does not allow it

Why shouldn't feat: Add something work with semantic release? Looks totally fine to me.

I'm also not sure what that log represents? It seems like you're using a different Github action there.

What is it exactly that you want to achieve and what behaviour of this action doesn't allow you to do that?

@denis-ev
Copy link
Author

denis-ev commented Jan 5, 2021

Hey @amannn,
when you read through https://www.conventionalcommits.org/en/v1.0.0/ everything is always lowercase and others implemented it so the lowercase part is forced. Your action works perfectly, just my commit lint check fails because some People create PRs starting the description with an upper case letter, which always fails with the commit lint when squashing.

It would be great if you could do that too, or if its an optional ENV, that might help others too.

- uses: amannn/action-semantic-pull-request@v2.1.0
        env:
          GITHUB_TOKEN: ${{ secrets.DE_PAT }}
          subject-case: lower or
          SUBJECT_CASE_LOW_FORCED: true

@amannn
Copy link
Owner

amannn commented Jan 5, 2021

Thanks for the additional information!

when you read through https://www.conventionalcommits.org/en/v1.0.0/ everything is always lowercase

Interesting, you're right – the description always starts lowercase.

There's however one exception:

docs: correct spelling of CHANGELOG

Are you sure that you'd also want to disallow this?

I had a look at the Conventional Commits spec and it seems like there are no restrictions on what characters the description can contain.

I guess what we could do is to allow for providing an additional env param (like you suggested), but I think I'd make it a bit more generic by allowing a regex.

E.g. to disallow uppercase characters at the beginning of the description you could configure:

descriptionPattern: ^(?![A-Z])

@denis-ev
Copy link
Author

denis-ev commented Jan 5, 2021

Well, I do have uppercase letters in mine too, mine just fails sometimes when the description starts uppercase.

I love the descriptionPattern: (?![A-Z]) idea.

That be awesome if you could do that.

btw this is a project, I'm mainly use it with https://github.com/VivumLab/VivumLab

If this regex works, I'm happy. haha

@amannn
Copy link
Owner

amannn commented Jan 5, 2021

Ok, I see – then it's only about the first character of the description. The pattern will validate against the description only, so this should do: https://regex101.com/r/D1cmbd/9

@denis-ev
Copy link
Author

denis-ev commented Jan 5, 2021

Ok, I see – then it's only about the first character of the description. The pattern will validate against the description only, so this should do: https://regex101.com/r/D1cmbd/9

Well that works for me too. Awesome, thanks.

@amannn amannn changed the title Check needs to be a bit more accurate Regex validation for description Jan 6, 2021
@amannn
Copy link
Owner

amannn commented Jan 7, 2021

Hey @denis-ev,

I think I've got a working implementation of this feature in #71. Would you like to help with reviewing the PR? I went for the term "subject" now instead of "description", because that's what the conventional commits parser uses as well.

@denis-ev
Copy link
Author

denis-ev commented Jan 7, 2021

@amannn
yes, I'll gladly test that.
It's 1:30am right now, but I'll test it tomorrow.

@amannn
Copy link
Owner

amannn commented Jan 7, 2021

Oh, don't worry – take your time 😃.

I already tested it a bit and it seems to work as expected. I'm not sure if there's any way how you'd be able to test the state of the pending PR to be honest. But a second pair of eyes on the code could be helpful if you like 🙂.

@denis-ev
Copy link
Author

denis-ev commented Jan 8, 2021

Oh, don't worry – take your time 😃.

I already tested it a bit and it seems to work as expected. I'm not sure if there's any way how you'd be able to test the state of the pending PR to be honest. But a second pair of eyes on the code could be helpful if you like 🙂.

Haha, yeah you can easily test it. This works really well btw.
https://github.com/denis-ev/Testing/pulls

name: "Lint PR"

on:
  pull_request_target:
    types:
      - opened
      - edited
      - synchronize

jobs:
  main:
    runs-on: ubuntu-latest
    steps:
      - uses: amannn/action-semantic-pull-request@feat/69-description-regex-validation
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        # Optionally, you can provide options for further constraints.
        with:
          # Configure additional validation for the subject based on a regex.
          # This example ensures the subject doesn't start with an uppercase character.
          subjectPattern: ^(?![A-Z]).+$

@amannn
Copy link
Owner

amannn commented Jan 8, 2021

amannn/action-semantic-pull-request@feat/69-description-regex-validation

Oh right, totally forgot about that! Good idea 🙂

@amannn
Copy link
Owner

amannn commented Jan 11, 2021

@denis-ev So, good to go from your perspective then?

@denis-ev
Copy link
Author

denis-ev commented Jan 11, 2021 via email

@amannn
Copy link
Owner

amannn commented Jan 11, 2021

Ok, thanks! Just released this in v.3.1.0.

@denis-ev
Copy link
Author

denis-ev commented Jan 11, 2021 via email

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