While evaluating this module, while it has a readme, I noticed the readme could use some helpful links and configurations. Also, the formatting does not align to Drupal standards for documentation. There should be a clear table of contents and lines should be hard wrapped at 80 characters for better accessibility.

According to Drupal standards, modules should include a useful README.file.
https://www.drupal.org/docs/develop/documenting-your-project/module-docu....

https://www.drupal.org/docs/develop/documenting-your-project/readme-temp...

Patch to follow, thanks!

tagging for Virtual Contrib Day

CommentFileSizeAuthor
#14 field_group_label-readme-2996544-13.patch1.67 KBdhirendra.mishra
#14 interdiff_11-13.txt232 bytesdhirendra.mishra
#9 field_group_label-readme-2996544-9.patch1.7 KBjohnny_aroza
#9 interdiff_4-9.txt180 bytesjohnny_aroza
#4 field_group_label-readme-2996544-4.patch1.72 KBjohnny_aroza
#4 interdiff_2-4.txt811 bytesjohnny_aroza
#2 field_group_label-readme-2996544-2.patch1.7 KBvolkswagenchick
#6 interdiff_4-6.txt180 bytesjohnny_aroza
#6 field_group_label-readme-2996544-6.patch1.7 KBjohnny_aroza
#11 interdiff_6-11.txt257 bytesjohnny_aroza
#11 field_group_label-readme-2996544-11.patch1.67 KBjohnny_aroza

Comments

volkswagenchick created an issue. See original summary.

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Assigned: volkswagenchick » Unassigned
Status: Active » Needs review
StatusFileSize
new field_group_label-readme-2996544-2.patch1.7 KB

I uploaded a patch that contains a useful readme that aligns to Drupal documentation.

jaykandari’s picture

jaykandari
Primary language Hindi
commented
Status: Needs review » Needs work

Thanks @volkswagenchick for the patch.

Patch #2 Applies correctly. There are few minor things I think needs work. 

  1. +++ b/README.txt
@@ -1,9 +1,53 @@
+ * Configuration

    Can we either remove this from Table of contents.
    OR
    Provide a section below describing how to configure this module? 

  2. +++ b/README.txt
@@ -1,9 +1,53 @@
+------------
...
+Supporting organization:
+

    1. Can we also add this section in Table of content section of this Readme?

    2. Can we have a ------------ appear below this title. Also if this can be capitalized. 

  3. +++ b/README.txt
@@ -1,9 +1,53 @@
+ * Install the Field Group Label module as you would normally install a

    Can we remove the little asterisk as to make this section appear as paragraph instead of bulleted list?

Thanks!

johnny_aroza’s picture

johnny_aroza commented
Assigned: Unassigned » johnny_aroza
Status: Needs work » Needs review
StatusFileSize
new field_group_label-readme-2996544-4.patch1.72 KB
new interdiff_2-4.txt811 bytes

jaykandari i have made changes according the inputs in all 3 points

Thank you

imalabya’s picture

imalabya
Primary language English
Location Bangalore
commented
Status: Needs review » Needs work 
+++ b/README.txt
@@ -1,9 +1,54 @@
+ * Supporting Organization

@JayKandari @johnny_aroza I don't think we need to add `Supporting organisation` as a heading as per https://www.drupal.org/docs/develop/documenting-your-project/readme-temp...

Apart from that rest looks good to me.

johnny_aroza’s picture

johnny_aroza commented
Status: Needs work » Needs review
StatusFileSize
new field_group_label-readme-2996544-6.patch1.7 KB
new interdiff_4-6.txt180 bytes

imalabya i removed Supporting Organization

imalabya’s picture

imalabya
Primary language English
Location Bangalore
commented 
+++ b/README.txt
@@ -1,9 +1,53 @@
+SUPPORTING ORGANIZATION
+-----------------------

This need not be a heading. Please revert it to the previous patch. Rest of it looks good to me.

imalabya’s picture

imalabya
Primary language English
Location Bangalore
commented
Status: Needs review » Needs work
johnny_aroza’s picture

johnny_aroza commented
Status: Needs work » Needs review
StatusFileSize
new field_group_label-readme-2996544-9.patch1.7 KB
new interdiff_4-9.txt180 bytes

imalabyai haved removed the heading

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Status: Needs review » Needs work 
+++ b/README.txt
@@ -1,9 +1,53 @@
+SUPPORTING ORGANIZATION
+-----------------------

This should not be a heading either :)

johnny_aroza’s picture

johnny_aroza commented
Status: Needs work » Needs review
StatusFileSize
new field_group_label-readme-2996544-11.patch1.67 KB
new interdiff_6-11.txt257 bytes

volkswagenchick i have removed the the heading .

Thank you

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Status: Needs review » Needs work 
+++ b/README.txt
@@ -1,9 +1,52 @@
+supporting organization

supporting organization => Supporting organization

dhirendra.mishra’s picture

dhirendra.mishra commented
Assigned: johnny_aroza » dhirendra.mishra
dhirendra.mishra’s picture

dhirendra.mishra commented
Assigned: dhirendra.mishra » Unassigned
Status: Needs work » Needs review
StatusFileSize
new interdiff_11-13.txt232 bytes
new field_group_label-readme-2996544-13.patch1.67 KB

Correcting the text mentioned in #12 and uploading the interdiff file from #11 and patch file. kindly test and review it.

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Status: Needs review » Reviewed & tested by the community

Looks good, thanks for the fix. Patch applied using simplytest.me

Marking as RTBC

crzdev’s picture

crzdev commented
Status: Reviewed & tested by the community » Fixed

Seems okay for me, thanks.

Status: Fixed » Closed (fixed)

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