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

Integrated internal changes from Google #2403

Merged
merged 18 commits into from
Nov 23, 2016
Merged

Conversation

acozzette
Copy link
Member

No description provided.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

A new test is now passing and so we can remove it from the failure
whitelist.
@xfxyjwf
Copy link
Contributor

xfxyjwf commented Nov 22, 2016

LGTM

The curly brace syntax for sets was introduced in Python 2.7, and so for
compatibility with 2.6 we need to avoid using it for now.
This seems to be necessary to prevent warnings in some compiler
configurations, particularly for tag numbers that are too large to fit
in a signed 32-bit int.
@acozzette acozzette force-pushed the down-integrate-with-msvc-fix branch from 1f67e56 to db35fe7 Compare November 23, 2016 00:24
In Python 3 the values() method on a dictionary returns a view instead
of a list, so we need to explicitly convert that to a list.
This makes a couple of changes to fix the tests for JRuby 1.7:
- Avoid using assert_false since that assertion seems not to exist in
  older versions
- Disable a test related to respond_to? for JRuby. It's hard to tell
  what is going wrong here but it looks like probably a JRuby bug that
  has been fixed in more recent versions.
@acozzette
Copy link
Member Author

@xfxyjwf I added a few more commits so do you want to take another look before I merge this? Now I believe all the tests should be passing. The CLA bot is confused but I think that shouldn't be a problem since and you and I are the only authors of commits in this pull request.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Nov 23, 2016

Thanks Adam. The changes all look good to me.

@acozzette
Copy link
Member Author

Thanks, Feng! I'll merge this now then.

@acozzette acozzette merged commit 39f9b43 into master Nov 23, 2016
@acozzette acozzette deleted the down-integrate-with-msvc-fix branch November 23, 2016 19:28
bithium pushed a commit to bithium/protobuf that referenced this pull request Sep 4, 2023
…th-msvc-fix

Integrated internal changes from Google
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