Display an AMI ID in AWS Launch Template listing view if the AMI name is not available

CommentFileSizeAuthor
#12 3200885-12.diff1.87 KBsekinet
#9 3200885-9.diff1.53 KBsekinet
#6 3200885-6.diff1.28 KBsekinet
#3 3200885-3.diff934 bytessekinet

Issue fork cloud-3200885

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sekinet created an issue. See original summary.

sekinet opened merge request !164

sekinet’s picture

sekinet CreditAttribution: sekinet commented
FileSize
3200885-3.diff934 bytes
sekinet’s picture

sekinet CreditAttribution: sekinet commented
Status: Active » Needs review

@yas I' ve created a patch, can you check it for me?

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs review » Needs work

@sekinet

Thank you for the refactoring. Can you add a link to $image_id? Please review my above comment. Thanks

sekinet’s picture

sekinet CreditAttribution: sekinet commented
FileSize
3200885-6.diff1.28 KB
sekinet’s picture

sekinet CreditAttribution: sekinet commented
Status: Needs work » Needs review

@yas Thank you for your comments. We have verified the code you gave us and it works fine.
Could you check the patch for me?

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs review » Needs work

@sekinet

Thank you for the update. I refactored the code. On my second thought, probably the first code that you implemented was correct, but can you please review the revised code again? Thanks

sekinet’s picture

sekinet CreditAttribution: sekinet commented
FileSize
3200885-9.diff1.53 KB
sekinet’s picture

sekinet CreditAttribution: sekinet commented
Status: Needs work » Needs review

@yas Thank you for your comments. The first code did not create the link and needed to be modified.
We have checked the code you gave us and it works fine.
Could you please check the patch?

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs review » Needs work

@sekinet

Thank you for the update. Your patch is better than my suggestion however please check my above comment. Thanks

sekinet’s picture

sekinet CreditAttribution: sekinet commented
FileSize
3200885-12.diff1.87 KB
sekinet’s picture

sekinet CreditAttribution: sekinet commented
Status: Needs work » Needs review

@yas Thank you for your comments.
I've created a patch, could you please check it?

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs review » Reviewed & tested by the community

@sekinet

Thank you for the update. I'll merge the patch to 8.x-1.x, 8.x-2.x and 3.x and close this issue as Fixed.

  • yas committed d9cb635 on 3.x authored by sekinet 
    Issue #3200885 by sekinet, yas: Display an AMI ID in AWS Launch Template...

  • yas committed 54b214e on 8.x-1.x authored by sekinet 
    Issue #3200885 by sekinet, yas: Display an AMI ID in AWS Launch Template...

  • yas committed 540b442 on 8.x-2.x authored by sekinet 
    Issue #3200885 by sekinet, yas: Display an AMI ID in AWS Launch Template...
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Reviewed & tested by the community » Fixed

yas closed merge request !164

Status: Fixed » Closed (fixed)

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