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

fix: third party enums don't break first class enums #118

Merged
merged 1 commit into from
Sep 10, 2020
Merged

fix: third party enums don't break first class enums #118

merged 1 commit into from
Sep 10, 2020

Conversation

software-dov
Copy link
Contributor

@software-dov software-dov commented Sep 4, 2020

For real, this time.
Third party, unwrapped enums no longer cause problems
if they are used to describe proto-plus fields.

Limitations:

  • Third party enums cannot be passed by string name to a Field

  • More scenarios require a module definition than previously
    ** Defining a proto.Enum may cause a FileDescriptor to be added to the
    descriptor pool. The only way to defer until all Enums and
    Messages in a module have been added is to provide a manifest or
    make the Enum a nested type of a Message.

    This mostly affects unit tests and interactive development.

Includes re-addition of stringy enums in JSON.
Includes re-addition of fix for #103

Release-As: 1.10.0-dev1

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 4, 2020
@busunkim96 busunkim96 linked an issue Sep 8, 2020 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Sep 8, 2020

Codecov Report

Merging #118 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #118   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           20        20           
  Lines          811       857   +46     
  Branches       136       149   +13     
=========================================
+ Hits           811       857   +46     
Impacted Files Coverage Δ
proto/_file_info.py 100.00% <100.00%> (ø)
proto/enums.py 100.00% <100.00%> (ø)
proto/fields.py 100.00% <100.00%> (ø)
proto/message.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update faf1992...f8498a7. Read the comment docs.

@software-dov
Copy link
Contributor Author

Python-dlp passes all unit tests using both the python and cpp implementations.

For real, this time.
Third party, unwrapped enums no longer cause problems
if they are used to describe proto-plus fields.

Limitations:
* Third party enums cannot be passed by string name to a Field
* More scenarios require a module definition than previously
** Defining a proto.Enum may cause a FileDescriptor to be added to the
   descriptor pool. The only way to defer until _all_ Enums and
   Messages in a module have been added is to provide a manifest or
   make the Enum a nested type of a Message.

   This mostly affects unit tests and interactive development.

Includes re-addition of stringy enums in JSON.
Includes re-addition of fix for #103

Release-As: 1.10.0-dev1
@software-dov software-dov merged commit 50b87af into googleapis:master Sep 10, 2020
@software-dov software-dov deleted the third-party-enums branch September 10, 2020 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1.9.0 breaking firestore unit tests
3 participants