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

Allow extra whitespace in merge commands #108

Merged
merged 3 commits into from
May 2, 2022

Conversation

ReinierMaas
Copy link

XRef: #102

it might also be more user-friendly to accept one or more spaces between the mention and the command instead of just one. ~ Riley

Yes, that is definitely more user-friendly!


Allowing extra whitespace in the merge commands to overcome the (somewhat) common double whitespace.

@ReinierMaas ReinierMaas added the enhancement New feature or request label Apr 21, 2022
@ReinierMaas ReinierMaas self-assigned this Apr 21, 2022
Copy link

@RileyApeldoorn RileyApeldoorn left a comment

Choose a reason for hiding this comment

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

Looks good! I left a small idea for improvement, but it's nothing critical so feel free to disregard it :)

src/Logic.hs Outdated
@@ -396,7 +396,7 @@ isSuccess _ = False
parseMergeCommand :: TriggerConfiguration -> Text -> ParseResult (ApprovedFor, MergeWindow)
parseMergeCommand config message =
let
messageCaseFold = Text.toCaseFold $ Text.strip message
messageCaseFold = Text.toCaseFold $ Text.unwords $ filter (not . Text.null) $ Text.words message

Choose a reason for hiding this comment

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

Minor nitpick: it might be nice to make a normalize function to group these transformations in (and maybe rename messageCaseFold to messageNormalized since case folding is not the only operation applied to it anymore) but that comes down to stylistic preference.

@ReinierMaas
Copy link
Author

@OpsBotPrime merge!

@OpsBotPrime
Copy link

Your merge request has been denied, because merging on Fridays is not recommended. To override this behaviour use the command merge on Friday.

@ReinierMaas
Copy link
Author

@OpsBotPrime merge!

@OpsBotPrime
Copy link

Pull request approved for merge by @ReinierMaas, rebasing now.

@OpsBotPrime
Copy link

Rebased as 6f1b2a3, waiting for CI …

OpsBotPrime added a commit that referenced this pull request May 2, 2022
Approved-by: ReinierMaas
Auto-deploy: false
@bertptrs bertptrs closed this May 2, 2022
@OpsBotPrime
Copy link

Abandoning this pull request because it was closed.

@bertptrs bertptrs reopened this May 2, 2022
@bertptrs
Copy link

bertptrs commented May 2, 2022

@OpsBotPrime merge and for real this time.

@ReinierMaas the CI was never started due to Semaphore issues, I'll retry this for you.

@OpsBotPrime
Copy link

Pull request approved for merge by @bertptrs, rebasing now.

ReinierMaas and others added 3 commits May 2, 2022 11:40
* Frees disk space on Semaphore
* Error on `nix-build` failures
Approved-by: bertptrs
Auto-deploy: false
@OpsBotPrime
Copy link

Rebased as 8cc81f5, waiting for CI …

@OpsBotPrime OpsBotPrime force-pushed the command-parser-allow-whitespace branch from e3641a0 to 8cc81f5 Compare May 2, 2022 09:47
@OpsBotPrime OpsBotPrime merged commit 8cc81f5 into master May 2, 2022
@OpsBotPrime OpsBotPrime deleted the command-parser-allow-whitespace branch May 2, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants