FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

SupportedRegion as a StrEnum by davidbuckman · Pull Request #112 · firebase/firebase-functions-python · GitHub

SupportedRegion as a StrEnum - #112

Closed
davidbuckman wants to merge 2 commits into
firebase:mainfrom
davidbuckman:supported-region-as-a-str-enum
Closed

SupportedRegion as a StrEnum#112
davidbuckman wants to merge 2 commits into
firebase:mainfrom
davidbuckman:supported-region-as-a-str-enum

Conversation

davidbuckman commented Jul 1, 2023
edited
Loading

Copy link
Copy Markdown

SupportedRegion should be a StrEnum so that __str__ works as expected, e.g. in

https://github.com/firebase/functions-samples/blob/main/Python/taskqueues-backup-images/functions/main.py#L144-L168

google-cla Bot commented Jul 1, 2023

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

russellwheatley commented Jul 10, 2023
edited
Loading

Copy link
Copy Markdown
Member

Thanks @davidbuckman, do you mind writing a few tests along the line of:

def test_supported_region_enum():
    """
    Testing SupportedRegion enum
    """
    assert options.SupportedRegion.EUROPE_WEST2 == "europe-west2"

in this file, please? https://github.com/firebase/firebase-functions-python/blob/main/tests/test_options.py

davidbuckman force-pushed the supported-region-as-a-str-enum branch from cc6090b to edf5495 Compare July 20, 2023 02:33

exaby73 commented Sep 11, 2023

Copy link
Copy Markdown
Contributor

Hello @davidbuckman. Could you run the commands for formatting and fix the linting and push so that the CI is green? You can find these commands on the contributing guidelines page

Copy link
Copy Markdown
Author

@russellwheatley anything else needed here?

davidbuckman force-pushed the supported-region-as-a-str-enum branch from cbc91a1 to baed9ed Compare January 29, 2024 14:14

exaby73 commented Feb 16, 2024

Copy link
Copy Markdown
Contributor

@davidbuckman since StrEnum was added in 3.11, and we are currently on 3.12 but supporting 3.10, this would be a breaking change. We've decided for now to just override the __str__ method manually on all enum classes, with the this PR being merged in on the next major version bump. For now, I'll leave this PR open and come back to it at a later date. Sorry for the inconvenience

exaby73 commented Mar 18, 2024

Copy link
Copy Markdown
Contributor

Superseded by #178

exaby73 closed this Mar 18, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL