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.

Sign up for GitHub

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

Jump to bottom

fix: improve regex matching #91

Merged
merged 2 commits into from
May 18, 2023
Merged

fix: improve regex matching #91

merged 2 commits into from
May 18, 2023

Conversation

ben-wilson-peak
Copy link
Contributor

@ben-wilson-peak ben-wilson-peak commented Apr 19, 2023 •
edited
Loading

Github comments can appear as \r\n, this improves the regex to support that new requirement

@ben-wilson-peak
Remove \n\n requirement
16ab4f5 
Removes \n\n requirement since github publishes comments with \r\n which breaks this search and replace. Functionally this doesn't cause breaking changes
@ben-wilson-peak
Copy link
Contributor Author

@bkeepers would you be willing to review this?

@ben-wilson-peak
Copy link
Contributor Author

@gr2m do you know who could be a reviewer for this?

@gr2m
Copy link
Contributor

gr2m commented May 16, 2023

sorry we are very short on maintenance time on @probot right now, and the metadata extension is defacto unmaintained. But this seems simple enough, I'm happy to merge it in for you

@gr2m
Copy link
Contributor

gr2m commented May 16, 2023

the failing tests seem legit. Tests pass on master when running npm test in Node 16, but fail in your branch. Can you look into that?

@ben-wilson-peak
Copy link
Contributor Author

the failing tests seem legit. Tests pass on master when running npm test in Node 16, but fail in your branch. Can you look into that?

Certainly, I'll see if I can sort that.

sorry we are very short on maintenance time on @probot right now, and the metadata extension is defacto unmaintained. But this seems simple enough, I'm happy to merge it in for you

Appreciated that you've responded more so knowing this!

@ben-wilson-peak ben-wilson-peak changed the title Remove \n\n requirement Improve regex matching May 17, 2023
@ben-wilson-peak
Copy link
Contributor Author

@gr2m I've updated the PR now. The tests were failing due to the change so I've instead opted to fix the regex matching and have updated the code accordingly. The tests remain unchanged.

@gr2m gr2m changed the title Improve regex matching fix: improve regex matching May 18, 2023
@gr2m gr2m merged commit a0d7ab2 into probot:master May 18, 2023
@github-actions
Copy link

🎉 This PR is included in version 2.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ben-wilson-peak ben-wilson-peak deleted the patch-1 branch May 18, 2023 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ben-wilson-peak @gr2m