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

Remove GTEST_HAS_STD_STRING #2456

Merged

Conversation

kuzkry
Copy link
Contributor

@kuzkry kuzkry commented Sep 10, 2019

1st commit removes GTEST_HAS_STD_STRING. I think we can drop it, after all it's 2019 :)
2nd commit simply removes usings from the header, which is how all guidelines say

Both changes are backward incompatible. I would merge them though because README.md says the following about post 1.8.x releases:

On-going work to improve/cleanup/pay technical debt

and we don't quite need it, right?

@kuzkry kuzkry force-pushed the gtest-port-clean-up_breaking-changes branch 5 times, most recently from 29e9f27 to 998732d Compare September 28, 2019 08:52
@gennadiycivil
Copy link
Contributor

273567737
Thank you, we have started internal review. Please don't push any more changes into this PR as they might be overwritten.

@gennadiycivil gennadiycivil self-assigned this Oct 8, 2019
@gennadiycivil
Copy link
Contributor

@kuzkry
So I run some internal tests, GTEST_HAS_STD_STRING looks OK ( I think but not 100% sure because of all of the noise from the other change ).

There is another change in this PR, ::testing::tuple_size -> std::tuple_size. This one looks harder because there is internal code accumulated over the years that is still using ::testing::tuple_size.
May I suggest splitting these two changes into two separate PRs?
Thank you!
G

@kuzkry kuzkry force-pushed the gtest-port-clean-up_breaking-changes branch from 998732d to 838ea5c Compare October 10, 2019 14:54
@kuzkry
Copy link
Contributor Author

kuzkry commented Oct 10, 2019

Sure, done.

@kuzkry kuzkry changed the title Remove unneeded stuff from gtest-port.h Remove GTEST_HAS_STD_STRING Oct 10, 2019
@gennadiycivil
Copy link
Contributor

Trying again
273993693
Thank you, we have started internal review. Please don't push any more changes into this PR as they might be overwritten.

gennadiycivil added a commit that referenced this pull request Oct 11, 2019
@gennadiycivil gennadiycivil merged commit 838ea5c into google:master Oct 11, 2019
@kuzkry kuzkry deleted the gtest-port-clean-up_breaking-changes branch October 11, 2019 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants