Problem/Motivation

  • Fix the description of fields on hover in the form element by making it a full sentence. (e.g., The name of launch template → Enter the name of launch template)
  • The title of fields in the form element should have only the first word capitalized unless the word is usually used that way. (e.g., Max Count → Max count)
CommentFileSizeAuthor
#13 aws_cloud_launch_template.png60.71 KBsekinet
#13 k8s_launch_template.png66.45 KBsekinet

Issue fork cloud-3238827

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

TamakiFujino created an issue. See original summary.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Issue summary: View changes

TamakiFujino opened merge request !424

TamakiFujino’s picture

TamakiFujino commented
Status: Active » Needs review
TamakiFujino’s picture

TamakiFujino commented
Status: Needs review » Needs work
TamakiFujino’s picture

TamakiFujino commented
Status: Needs work » Needs review
TamakiFujino’s picture

TamakiFujino commented
Title: Fix the UI of AWS Cloud Instances (1) (Form Elements) » Fix the UI of AWS Cloud Instances (1.1) (Form Elements)
TamakiFujino’s picture

TamakiFujino commented
Status: Needs review » Needs work
TamakiFujino’s picture

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

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

@tamakifujino

Thank you for the refactoring. I posted my comments above. Could you please check it?

@baldwinlouie, @xiaohua-guan, @kumikoono, @sekinet, @lidia, @yutong.li

What do you think?

xiaohua guan’s picture

xiaohua guan commented

@yas

The code looks good to me.

baldwinlouie’s picture

baldwinlouie commented

@tamkifujino. Thank you for the patch. I posted my comments.

sekinet’s picture

sekinet commented
StatusFileSize
new k8s_launch_template.png66.45 KB
new aws_cloud_launch_template.png60.71 KB

@tamkifujino @yas

I checked the launch template and it looks like there are a few capital letters left in it.
Can you please check the following image?

aws_cloud

k8s

TamakiFujino’s picture

TamakiFujino commented

@sekinet @yas

Similar to the comment to Yutong's but I think I have changed "Source Type" to "Source type" and "Git Resource Path" to "Git resource path."
I think changes are not reflected on the website yet.

Also, for "Availability Zone" in docs.aws.amazon.com they use "Availability Zone" instead of "availability zone." So, I left it as it is. Should I de-capitalize it too?

yas’s picture

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

@tamakifujino (cc: @sekinet)

We can keep Availability Zone since AWS uses so.

TamakiFujino’s picture

TamakiFujino commented
Status: Needs work » Needs review

kumikoono made their first commit to this issue’s fork.

kumikoono’s picture

kumikoono commented

@TamakiFujino, @baldwinlouie, @yas Can you review the changes in .install files?

baldwinlouie’s picture

baldwinlouie commented

@kumikoono Thank you for the changes to the install file. It looks good. I left one additional comment. Basically, we need a hook_update_N() for Openstack module as well.

yas’s picture

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

@tamakifujino

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

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Title: Fix the UI of AWS Cloud Instances (1.1) (Form Elements) » Fix the UI of AWS Cloud Instances (Form Elements)
yas’s picture

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

@tamakifujino

I tried to apply the patch however got the following error:

error: patch failed: modules/cloud_service_providers/openstack/openstack.install:1290
error: modules/cloud_service_providers/openstack/openstack.install: patch does not apply

So could you please double-check the patch?

Thanks!

yas’s picture

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

@tamakifujino

Thank you for the update. It looks good. I'll merge the patch to 4.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

@tamakifujino

Sorry for revisiting again. I found the following coding standard violations. Could you please fix that? Thanks!

$ ./vendor/bin/phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml "${CODE_DIR}"
FILE: ...loud/modules/cloud_service_providers/openstack/openstack.install
----------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------
 1293 | ERROR | [x] Expected 1 blank line after function; 2 found
 1300 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
      |       |     found 4
 1303 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
      |       |     found 4
 1304 | ERROR | [x] Expected 1 newline at end of file; 3 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: ...nnovations/cloud/modules/cloud_service_providers/k8s/k8s.install
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 3403 | ERROR | [x] Expected 1 blank line after function; 0 found
 3416 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
Time: 33.77 secs; Memory: 54.01MB
TamakiFujino’s picture

TamakiFujino 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

@tamakifujino

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

  • yas committed 0afb2b5 on 4.x authored by TamakiFujino 
    Issue #3238827 by TamakiFujino, kumikoono, sekinet, yas, baldwinlouie,...

yas closed merge request !424

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.