-
-
Notifications
You must be signed in to change notification settings - Fork 21.9k
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
Add additional tests for RegEx #81742
Add additional tests for RegEx #81742
Conversation
Also please squash your commits into one, see here Make sure you push your changes with |
c194473
to
23b068f
Compare
You erased your previous fixes, please add them back in and squash all commits into one |
d811ea0
to
c4fede7
Compare
These changes should be as per the suggestions. Let me know if there are more changes. |
why ask developers to manually squash into one commit when github can do it for you on merge of PR by checking a box under settings? |
Because we don't do those features, they add their own artifacts and can create issues as far as I know |
interesting, we use that feature at work and i have not seen any issues, but ill take your word for it |
We don't use GitHub's squash and merge feature on the main Godot repository, since it loses the merge commit in the process. We prefer keeping the merge commit to make the Git history more self-documented (even if that makes it longer when not using the |
c4fede7
to
b7de8e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally (rebased on top of master
83b916b), it works as expected.
I force-pushed a rebased version with the commit message modified to follow conventions used in the repository, so this should be good to merge now.
Thanks! And congrats for your first merged Godot contribution 🎉 |
This PR should hopefully address more tests for RegEx. This task is part of #43440.
More Details: