Problem/Motivation

Refactor CloudStore as following:

  • Store costs of all cost types in one field for K8s Resource Store entities
  • Add the duration for CloudConfig to keep the CloudStore
  • Remove field_cloud_context from CloudStore and use cloud_context
  • Refactor costs field and resources field of K8s Resource Store entities

Issue fork cloud-3221053

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

Masami created an issue. See original summary.

Masami opened merge request !287

masami’s picture

masami commented
Status: Active » Needs review
yas’s picture

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

@masami

Thank you for supplying the patch. I reviewed it and posted my comments. I think most of foreach() can be changed to array_map(). Also, please use the dependency injection if available.

Thanks

masami’s picture

masami commented
Status: Needs work » Needs review
yas’s picture

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

@masami

Thank you for the update. It looks good, and I posted my comments above for some cosmetic change. Thanks

masami’s picture

masami commented
Status: Needs work » Needs review
yas’s picture

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

@masami

Thank you for the update. I posted my additional comments above. Could you please check it? Thanks

yas’s picture

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

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

@masami

Thank you for the update. There is one leftover (remove $avg_cost = [];) and my one minor refactoring comment.

masami’s picture

masami 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

@masmai

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

  • yas committed 052f469 on 4.x authored by Masami 
    Issue #3221053 by Masami, yas: Refactor CloudStore

yas closed merge request !287

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.