Skip to content

Comments

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

Merged
derickr merged 1 commit intomongodb:masterfrom
derickr:PHPC-742-regex-default
Sep 7, 2016
Merged

PHPC-742: Regex constructor should default flags arg to empty string#388
derickr merged 1 commit intomongodb:masterfrom
derickr:PHPC-742-regex-default

Conversation

@derickr
Copy link
Contributor

@derickr derickr commented Sep 6, 2016

No description provided.

Copy link
Contributor

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
Contributor

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
Contributor

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