• Update OpenStackImageEditForm::save().
  • Refactor the code and prepare it for easy modification.

Issue fork cloud-3280044

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

Ryo Yamashita created an issue. See original summary.

Ryo Yamashita opened merge request !935

ryo yamashita’s picture

ryo yamashita commented
Status: Needs work » Needs review

@yas

Please review it. Thanks!

yas’s picture

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

@ryo-yamashita

Thank you for the patch. I posted my comments. Thanks

ryo yamashita’s picture

ryo yamashita commented
Status: Needs work » Needs review

@yas

I created CloudOperationsService and AwsCloudOperationsService.
It changed the place where functions are defined to a more correct position.
Please review it again. Thanks!

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented

@ryo-yamashita

Thank you for the update. It looks even better and makes more sense to me.

@xiaohua-guan @baldwinlouie

Could you please review the patch?

Thanks

xiaohua guan’s picture

xiaohua guan commented

@ryo-yamashita

Please fix the test failures.

ryo yamashita’s picture

ryo yamashita commented
Status: Needs review » Needs work

Sorry, I forgot that adding a DI (constructor argument) to class X also requires modifications to class Y, which inherits from it.

baldwinlouie’s picture

baldwinlouie commented

@ryo-yamashita, Thank you for the patch. Aside from the test failures, it looks good to me.

ryo yamashita’s picture

ryo yamashita commented
Status: Needs work » Needs review
yas’s picture

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

@ryo-yamashita

Thank you for the update. I posted my comment. Can you check it? Thanks

ryo yamashita’s picture

ryo yamashita commented
Status: Needs work » Needs review
yas’s picture

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

@ryo-yamashita

Thank you for the update. I'll merge the patch to 4.x and 5.x, and close this issue as Fixed.

  • yas committed 5d8e479 on 5.x authored by Ryo Yamashita 
    Issue #3280044 by Ryo Yamashita, yas, Xiaohua Guan, baldwinlouie:...

  • yas committed 82f9940 on 4.x authored by Ryo Yamashita 
    Issue #3280044 by Ryo Yamashita, yas, Xiaohua Guan, baldwinlouie:...
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.