Problem/Motivation

  • Typehinting will be required before drupal:10.0.0.
CommentFileSizeAuthor
#14 3191583-14.patch175.09 KBaadil.addweb
#12 3191583-12.patch175.04 KBaadil.addweb
#10 3191583-10.patch171.26 KBaadil.addweb
#8 3191583-8.patch170.21 KBaadil.addweb
#6 3191583-6.patch160.28 KBaadil.addweb

Issue fork cloud-3191583

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

yas created an issue. See original summary.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Related issues: +#3191576: Add function's return type declaration (AWS Cloud), +#3191580: Add function's return type declaration (Cloud), +#3191581: Add function's return type declaration (K8s), +#3191582: Add function's return type declaration (Docker), +#3191584: Add function's return type declaration (Terraform), +#3191585: Add function's return type declaration (VMware), +#3191586: Add function's return type declaration (GApps), +#3191587: Add function's return type declaration (Tools)
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Related issues: +#3191610: Add function's return type declaration (Cloud Budget)
aadil.addweb’s picture

aadil.addweb commented
Assigned: Unassigned » aadil.addweb

binoli.addweb opened merge request !135

aadil.addweb’s picture

aadil.addweb commented
Title: Add function's return type declaration (OpenStack) » 3191583-add-functions-return-type-declaration-openstack
Status: Active » Needs review
StatusFileSize
new 3191583-6.patch160.28 KB
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Title: 3191583-add-functions-return-type-declaration-openstack » Add function's return type declaration (OpenStack)
aadil.addweb’s picture

aadil.addweb commented
StatusFileSize
new 3191583-8.patch170.21 KB
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@binoliaddweb

Thank you for the refactoring. Did you have any reason why you didn't add the return type hint into the following methods?

  1. https://git.drupalcode.org/project/cloud/-/blob/3.x/modules/cloud_servic...
  2. https://git.drupalcode.org/project/cloud/-/blob/3.x/modules/cloud_servic...
  3. https://git.drupalcode.org/project/cloud/-/blob/3.x/modules/cloud_servic...
  4. https://git.drupalcode.org/project/cloud/-/blob/3.x/modules/cloud_servic...

Thanks

aadil.addweb’s picture

aadil.addweb commented
Status: Needs work » Needs review
StatusFileSize
new 3191583-10.patch171.26 KB

@yas

I've added return type in all above methods which you added in your previous comment.

We can't add here https://git.drupalcode.org/project/cloud/-/blob/3.x/modules/cloud_service_providers/openstack/src/Form/OpenStackInstanceEditForm.php#L383 as it returns mixed type (array or boolean).

Please review the updated patch.

Thanks

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@binoliaddweb

Thank you for the update. Please check my above comments. Thanks.

aadil.addweb’s picture

aadil.addweb commented
Status: Needs work » Needs review
StatusFileSize
new 3191583-12.patch175.04 KB

@yas

Thank you for your review.

I've changed the getFloatingIpByIpLookup method's logic in OpenStackInstanceEditForm.php.

Please review the updated patch.

Thanks

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@binoliaddweb

Thank you for the refactoring. It looks good to now but please modify the PhpDoc comment. Please see my above comment. Thanks

aadil.addweb’s picture

aadil.addweb commented
Status: Needs work » Needs review
StatusFileSize
new 3191583-14.patch175.09 KB

@yas

Thank you for your review.

I changed the Phpdoc comment. Please review the updated patch.

Thanks

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Reviewed & tested by the community

@binoliaddweb

Thank you for the update. Now everything looks good. I'll merge the patch to 3.x and close this issue as Fixed.

  • yas committed ccede28 on 3.x authored by binoli.addweb 
    Issue #3191583 by binoli.addweb, yas: Add function's return type...

yas closed merge request !135

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.