Update README.md

CommentFileSizeAuthor
#18 3017100-18.patch3.09 KByas
#13 3017100-13.patch5.94 KByas
#11 3017100-11.patch5.99 KByas
#6 3017100-6.patch5.96 KByas
#2 3017100-2.patch5.88 KByas
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yas created an issue. See original summary.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Active » Needs review
FileSize
3017100-2.patch5.88 KB

Status: Needs review » Needs work

The last submitted patch, 2: 3017100-2.patch, failed testing. View results

yas’s picture

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

Testing is failed but the file itself should be fine.

baldwinlouie’s picture

baldwinlouie CreditAttribution: baldwinlouie commented
Status: Needs review » Needs work 
+++ README.md	(date 1543532411000)
-- 512MB Memory: If you use AWS Cloud module, the running host of this
+- `baldwinlouie` (https://www.drupal.org/u/baldwinlouie)

We should add our new contributers too! @xiaohua-guan and @shidat

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs work » Needs review
FileSize
3017100-6.patch5.96 KB

Absolutely. I added them to the maintainers.

yas’s picture

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

Status: Needs review » Needs work

The last submitted patch, 6: 3017100-6.patch, failed testing. View results

yas’s picture

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

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

@yas, I made a couple of addition comments.

  1. +++ README.md	(date 1543620351000)
-- Future support includes OpenStack, Eucalyptus, Cloudn, Docker via Kubernetes
+sucn as Amazon Management Console, RightScale, ElasticFox, and etc.
...
 
...
-=======================================

    There is a typo. "sucn" should be "such" 

  2. +++ README.md	(date 1543620351000)
-- Future support includes OpenStack, Eucalyptus, Cloudn, Docker via Kubernetes
+1. `composer require aws/aws-sdk-php:dev-master`

    I think we should tell the users who do not use composer, how to install the aws-sdk. For example, download the SDK from https://docs.aws.amazon.com/aws-sdk-php/v3/download/aws.zip and unzip it into the "vendor" directory 

  3. +++ README.md	(date 1543620351000)
-- Enable the aws_cloud module.  This will also enable the required modules.
+   *You must add each AWS region as a separate `Cloud Config entity`*

    We should give an example of AWS region. (ie, us-west-1, us-west-2)

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs work » Needs review
FileSize
3017100-11.patch5.99 KB

@baldwinlouie

I included your comments. Could you please review it?

Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented

@yas

Here is only one comment. Please confirm it.

> +_OR_ (using `composer`)
> +
> +1. `composer require aws/aws-sdk-php:dev-master`
> +2. `composer require drupal/cloud`

Because aws/aws-sdk-php is included in the composer.json of module drupal, it is unnecessary to run `composer require aws/aws-sdk-php:dev-master`.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
FileSize
3017100-13.patch5.94 KB

@xiaohua-guan

Thank you for your review. I removed `composer require aws/aws-sdk-php:dev-master`.

baldwinlouie’s picture

baldwinlouie CreditAttribution: baldwinlouie commented
Status: Needs review » Reviewed & tested by the community

@yas, looks good. Moving to RTBC

  • yas committed 114fe6a on 8.x-1.x 
    Issue #3017100 by yas, baldwinlouie, Xiaohua Guan: Update README.md
yas’s picture

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

@baldwinlouie

Thank you for your review. I merged the patch and marked this issue as Fixed

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’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Fixed » Needs review
FileSize
3017100-18.patch3.09 KB

Fix the following coding standards errors.

FILE: ...000000/site1101/web/vendor/drupal/pareviewsh/pareview_temp/README.md
--------------------------------------------------------------------------
FOUND 1 ERROR AND 6 WARNINGS AFFECTING 7 LINES
--------------------------------------------------------------------------
  3 | WARNING | [ ] Line exceeds 80 characters; contains 83 characters
  5 | WARNING | [ ] Line exceeds 80 characters; contains 82 characters
 26 | WARNING | [ ] Line exceeds 80 characters; contains 90 characters
 44 | WARNING | [ ] Line exceeds 80 characters; contains 87 characters
 49 | WARNING | [ ] Line exceeds 80 characters; contains 81 characters
 55 | WARNING | [ ] Line exceeds 80 characters; contains 81 characters
 91 | ERROR   | [x] Expected 1 newline at end of file; 2 found
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

  • yas committed ace396c on 8.x-1.x 
    Issue #3017100 by yas, baldwinlouie, Xiaohua Guan: Update README.md -...
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

Status: Fixed » Closed (fixed)

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