Problem/Motivation

  • Implement OpenStackService to use OpenStack native API for Images List and Operations
CommentFileSizeAuthor
#30 3186241-30.patch6.75 KByas
#29 3186241-29.patch43.02 KByas
#23 3186241-23.patch43.02 KByas
#9 3186241-9.patch44.04 KByas

Issue fork cloud-3186241

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

binoli.addweb created an issue. See original summary.

binoli.addweb opened merge request !42

aadil.addweb’s picture

aadil.addweb commented
Title: Implement OpenStackService to use OpenStack native API (Images) (List and Operations) » 3186241-implement-openstackservice-to-use-openstack-native-api-for-images
Status: Active » Needs review
aadil.addweb’s picture

aadil.addweb commented
Title: 3186241-implement-openstackservice-to-use-openstack-native-api-for-images » Implement OpenStackService to use OpenStack native API (Images) (List and Operations)
yas’s picture

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

@binoliaddweb

I reviewed the MR. Please check my comments.

aadil.addweb’s picture

aadil.addweb commented
Status: Needs work » Needs review
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work
aadil.addweb’s picture

aadil.addweb commented
Status: Needs work » Needs review

@yas

Thank you for reviewing the patch.

I've refactored the code as per your comment of MR.

Please review.

Thanks

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work
StatusFileSize
new 3186241-9.patch44.04 KB
aadil.addweb’s picture

aadil.addweb commented
Status: Needs work » Needs review

@yas

Thank you for reviewing the code.

I refactored the comments in all Image forms.

Please review.

Thanks

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work
aadil.addweb’s picture

aadil.addweb commented
Status: Needs work » Needs review
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work
aadil.addweb’s picture

aadil.addweb commented
Status: Needs work » Needs review
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work
aadil.addweb’s picture

aadil.addweb commented
Status: Needs work » Needs review
yas’s picture

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

@binoliaddweb

Thank you for the update. However here are some confusion; so could you please double-check the code for $cloud_context, $entity->getCloudContext(), and $this->cloudContext and so on?

aadil.addweb’s picture

aadil.addweb 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

@binoliaddweb

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

yas’s picture

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

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

@binoliaddweb

Please re-create the patch to merge. Thanks.

aadil.addweb’s picture

aadil.addweb commented
Status: Needs work » Needs review

@yas

I've fixed the patch applying error and committed the code.

Please review.

Thanks

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 3186241-23.patch43.02 KB

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 23: 3186241-23.patch, failed testing. View results

  • yas committed a13b020 on 3.x authored by binoli.addweb 
    Issue #3186241 by binoli.addweb, yas: Implement OpenStackService to use...
yas’s picture

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

yas closed merge request !42

yas’s picture

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

@binoliaddweb

I found there were some codes to be refactored. Could you please fix it?

Thanks

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs work » Needs review
StatusFileSize
new 3186241-29.patch43.02 KB
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
StatusFileSize
new 3186241-30.patch6.75 KB
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 refactoring. It looks good, I'll merge the patch to 3.x and close this issue as Fixed.

  • yas committed 223d7ea on 3.x authored by binoli.addweb 
    Issue #3186241 by binoli.addweb, yas: Refactor OpenStackService to use...
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.