Skip to content

Conversation

@parthea
Copy link
Contributor

@parthea parthea commented Feb 12, 2025

Googlers see go/gapic-default-imports

@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Feb 12, 2025
@parthea parthea marked this pull request as ready for review February 12, 2025 23:20
@parthea parthea requested a review from a team as a code owner February 12, 2025 23:20
@parthea parthea requested review from ohmayr and vchudnov-g February 12, 2025 23:22

@pytest.mark.parametrize("unversioned_package_disabled", (True, False))
def test_get_response_ignore_unversioned_package(unversioned_package_disabled):
g = make_generator()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use one character variable names. I see the same construct is used in other tests, but I advocate not perpetuating the practice.

Suggested change
g = make_generator()
generator = make_generator()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 20f4050

service_yaml_config=service_config,
)

with mock.patch.object(jinja2.FileSystemLoader, "list_templates") as lt:
Copy link
Contributor

@vchudnov-g vchudnov-g Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd still suggest avoiding short names if possible, though these are two-character and used closed to the definition, which makes it more amenabel IMO.

Consider s/lt/lister/ and s/gt/getter/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6491e53

@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Feb 14, 2025
@parthea parthea enabled auto-merge (squash) February 14, 2025 00:10
@parthea parthea merged commit ccd619f into main Feb 14, 2025
126 checks passed
@parthea parthea deleted the allow-disabling-unversioned-packages branch February 14, 2025 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: l Pull request size is large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants