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

Chore: Improve PR title validation regex #24467

Merged
merged 4 commits into from Feb 17, 2022
Merged

Conversation

sampaiodiego
Copy link
Member

@sampaiodiego sampaiodiego commented Feb 10, 2022

Proposed changes (including videos or screenshots)

I don't know how it happened, but looks like the regex was accepting some invalid titles like for example:

[IMPROVE][EE] Description

In this case, it should be invalid because the seconds group ([EE]) is invalid, but since [IMPROVE] is valid, the regex has a match and so the title is considered valid.

The new regex will validate the second group as well, if present, and allow only the values [ENTERPRISE] and [APPS].

I've used this list of valid and invalid titles to check the regex:

[NEW] ok
[BREAK] ok
[IMPROVE] ok
[FIX] ok
[BREAK][ENTERPRISE] ok
[BREAK][APPS] ok
Regression: ok
Chore: ok
Revert: ok
i18n: ok
Bump ok
Release 1.2.3
Merge master into develop

[NEW]not ok
[NEW][NOPE] not ok
Regression:not ok
Bump: not ok
Release version 1.2.3 not ok

I'm considering having somes tests to make sure the regex works as expected and having a reference to check at some point. wdyt?

Issue(s)

Steps to test or reproduce

Further comments

@sampaiodiego sampaiodiego requested review from a team February 10, 2022 17:50
Co-authored-by: Debdut Chakraborty <debdut.chakraborty@rocket.chat>
Co-authored-by: Debdut Chakraborty <debdut.chakraborty@rocket.chat>
@sampaiodiego sampaiodiego merged commit 467261f into develop Feb 17, 2022
@sampaiodiego sampaiodiego deleted the improve-pr-title-regex branch February 17, 2022 21:49
@pierre-lehnen-rc pierre-lehnen-rc mentioned this pull request Mar 1, 2022
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.

None yet

2 participants