Skip to content

PHPC-742: Regex constructor should default flags arg to empty string #388

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

Merged
merged 1 commit into from
Sep 7, 2016

Conversation

derickr
Copy link
Contributor

@derickr derickr commented Sep 6, 2016

No description provided.

array("regex" => $regexp),
);

foreach($tests as $n => $test) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you have intention of adding additional test cases? If not, we can do away with this foreach.

Copy link
Contributor Author

@derickr derickr Sep 6, 2016

Choose a reason for hiding this comment

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

It's a copy of #001, which had the same thing. I didn't want to deviate (or change an existing test).

Copy link
Member

Choose a reason for hiding this comment

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

If you're merging this as-is, don't object if I go back and refactor this at some point. It's dead code that doesn't add any value to the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't mind!

@jmikola
Copy link
Member

jmikola commented Sep 6, 2016

LGTM with above comments.

I'll understand if you don't want to combine the omitted flag variations in the original test cases; however, I would like more descriptive test titles if we're keeping them split.

@derickr
Copy link
Contributor Author

derickr commented Sep 6, 2016

Will update the test descriptions and merge.

@derickr derickr force-pushed the PHPC-742-regex-default branch from 5e7a059 to 85e8328 Compare September 7, 2016 10:36
@derickr derickr force-pushed the PHPC-742-regex-default branch from 85e8328 to 213a8f0 Compare September 7, 2016 13:51
@derickr derickr merged commit 213a8f0 into mongodb:master Sep 7, 2016
derickr added a commit that referenced this pull request Sep 7, 2016
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.

2 participants