Skip to content
This repository has been archived by the owner on Jun 28, 2022. It is now read-only.

[PHP] Namespace inconsistency during deprecation #2141

Closed
tmatsuo opened this issue Jul 10, 2018 · 10 comments
Closed

[PHP] Namespace inconsistency during deprecation #2141

tmatsuo opened this issue Jul 10, 2018 · 10 comments
Assignees
Labels
lang: php Issues specific to PHP. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@tmatsuo
Copy link

tmatsuo commented Jul 10, 2018

We're trying to update the KMS client lib for PHP. The newly generated code is using a nicer namespace like Google\Cloud\Kms\V1\CryptoKey\CryptoKeyPurpose instead of Google\Cloud\Kms\V1\CryptoKey_CryptoKeyPurpose. Also the generator keeps the old namespaced class with a deprecation warning.

This is a great feature, but we found there are some usages of old classes in various places.

  1. Google\Cloud\Kms\V1\Gapic\KeyManagementServiceGapicClient
  2. \Google\Cloud\Kms\V1\CryptoKey.php
  3. Some occurrences in the unit tests \Google\Cloud\Kms\Tests\Unit\V1\KeyManagementServiceClientTest

For 1, this is the sample code in the doc comment, so it is basically telling users to use the deprecated class which will be removed with the next release. I think the sample code should use the newer class name.

For 2 and 3, I still think it is cleaner to use the newer namespace.

@michaelbausor michaelbausor added the lang: php Issues specific to PHP. label Aug 15, 2018
@michaelbausor michaelbausor self-assigned this Aug 15, 2018
@vchudnov-g vchudnov-g added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Nov 12, 2018
@vchudnov-g
Copy link
Contributor

@michaelbausor Updates?

1 similar comment
@andreamlin
Copy link
Contributor

@michaelbausor Updates?

@jbolinger
Copy link
Contributor

@michaelbausor any updates?

2 similar comments
@landrito
Copy link
Contributor

landrito commented Jan 2, 2019

@michaelbausor any updates?

@jbolinger
Copy link
Contributor

@michaelbausor any updates?

@michaelbausor
Copy link
Contributor

No update at this time.

@vchudnov-g
Copy link
Contributor

@michaelbausor Could you provide an update and/or lower the priority if appropriate?

@michaelbausor
Copy link
Contributor

This will be fixed in #2757

@JustinBeckwith JustinBeckwith added the 🚨 This issue needs some love. label Jun 27, 2019
@vchudnov-g
Copy link
Contributor

@hzyi-google Did #2757 fix this? If so could you close this issue?

@yihanzhen
Copy link
Contributor

Fixed by #2757.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lang: php Issues specific to PHP. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

8 participants