Comments

xaiwant created an issue. See original summary.

xaiwant’s picture

Assigned: xaiwant » Unassigned
Status: Needs work » Needs review
StatusFileSize
new1.48 KB

updated README.md attached updated patch

joshi.rohit100’s picture

+++ b/README.md
@@ -1,17 +1,42 @@
-# Remote image
+CONTENTS OF THIS FILE
+---------------------

content or contents ?

xaiwant’s picture

Hi @joshi.rohit100

Please refer below URL for documentation guidance.
https://www.drupal.org/node/2181737

Sonal.Sangale’s picture

Assigned: Unassigned » Sonal.Sangale
Status: Needs review » Reviewed & tested by the community
anavarre’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/README.md
    @@ -1,17 +1,42 @@
    +When building a site which leverages 3rd party APIs with images, you don't want to store those images
    

    We usually wrap lines at 80 cols.

  2. +++ b/README.md
    @@ -1,17 +1,42 @@
    +Therefore this module provides a custom field type, which allows you to store the URL to the image, as
    

    80 cols.

  3. +++ b/README.md
    @@ -1,17 +1,42 @@
    +Field module(Field API to add fields to entities like nodes and users.)
    

    s/module(/module (/

  4. +++ b/README.md
    @@ -1,17 +1,42 @@
    + * On successfull enable of the module, you will find a new field type "Remote Image" with
    

    "After the module has been successfully enabled, ..."

    "field of type ..."

    Also, 80 cols.

  5. +++ b/README.md
    @@ -1,17 +1,42 @@
    +   custom widget and fiedl formatter.
    

    "a custom widget and field formatter."

  6. +++ b/README.md
    @@ -1,17 +1,42 @@
    +Current maintainers:
    

    maintainer

anavarre’s picture

Assigned: Sonal.Sangale » Unassigned
harsha012’s picture

StatusFileSize
new1.54 KB

updated patch as per the comment #6

anavarre’s picture

  1. +++ b/README.md
    @@ -1,17 +1,45 @@
    + * Maintainers
    

    Thinking about this more, since we have only one maintainer, we should probably go with "Maintainer"

  2. +++ b/README.md
    @@ -1,17 +1,45 @@
    + * Field module (Field module to add fields to entities like nodes and users.)
    

    I suggest "Field (to add fields to entities like nodes and users.)" so that we don't repeat "module" over and over again.

  3. +++ b/README.md
    @@ -1,17 +1,45 @@
    + of type "Remote Image" with custom widget and field formatter.
    

    How about "a custom widget"?

  4. +++ b/README.md
    @@ -1,17 +1,45 @@
    +MAINTAINERS
    

    MAINTAINER

  5. +++ b/README.md
    @@ -1,17 +1,45 @@
    +Current maintainer:
    

    Per the above, we can drop this line completely.

harsha012’s picture

Status: Needs work » Needs review
StatusFileSize
new1.49 KB

@anavarre added the patch as per comment #8

anavarre’s picture

Status: Needs review » Needs work

Almost!

  1. +++ b/README.md
    @@ -1,17 +1,44 @@
    + * Maintainers
    

    Should be "Maintainer", or, if we keep "Maintainers", then the below "MAINTAINER" section needs to be pluralized.

  2. +++ b/README.md
    @@ -1,17 +1,44 @@
    +This module requires the following module:
    

    I'd simply reword this as "This module has a dependency on the core Field module, to add fields to entities, like nodes and users."

  3. +++ b/README.md
    @@ -1,17 +1,44 @@
    +   of type "Remote Image" with field formatter.
    

    "with a custom widget and field formatter."

Sonal.Sangale’s picture

Assigned: Unassigned » Sonal.Sangale
Status: Needs work » Needs review
StatusFileSize
new1.5 KB

Updated patch as per comment #11

anavarre’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/README.md
@@ -1,17 +1,42 @@
+This module has a dependency on the core Field module, to add fields to entities, like nodes and users.

This line should be wrapped at 80 cols.

Since this can easily fixed on commit, I'm tentatively marking it as RTBC, pending @dawehner's review. Thanks for sticking to it.

xaiwant’s picture

Assigned: Sonal.Sangale » Unassigned
StatusFileSize
new1.51 KB

@anavarre
line wrapped with 80 cols.

+++ b/README.md
+REQUIREMENTS
+------------
+This module has a dependency on the core Field module, to add fields to entities, 
+like nodes and users.
anavarre’s picture

+++ b/README.md
@@ -1,17 +1,43 @@
+This module has a dependency on the core Field module, to add fields to entities, ¶

Thanks, but you forgot a comma and invisible space at the end of line.

xaiwant’s picture

StatusFileSize
new1.51 KB

comma shifted to next line and removed invisible space at the end of line

anavarre’s picture

+++ b/README.md
@@ -1,17 +1,43 @@
+,like nodes and users.

Missing whitespace between "," and "like", but this can be fixed on commit.

renatog’s picture

StatusFileSize
new1.5 KB

-

renatog’s picture

StatusFileSize
new1.51 KB

Fixed @anavarre.

Patch it's in attachment.

Good Work.

Regards.

renatog’s picture

dawehner’s picture

Status: Reviewed & tested by the community » Fixed

Thank you @RenatoG!

Status: Fixed » Closed (fixed)

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