While evaluating this module, I noticed the README was very sparse.

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

README to follow. Thanks!

CommentFileSizeAuthor
#8 private_message-add-useful-readme-2911812-8.patch3.72 KBvolkswagenchick
#8 interdiff-2911812-2-8.txt1.44 KBvolkswagenchick
#2 private_message-adds-useful-readme-2911812-2.patch3.49 KBvolkswagenchick

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: Needs work » Needs review
StatusFileSize
new private_message-adds-useful-readme-2911812-2.patch3.49 KB

I uploaded a patch that includes a useful README with configurations, thanks!

dani3lr0se’s picture

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

Looks good. I was able to go through and follow setup/configuration steps and could find my way around easily. Thanks!

dhruveshdtripathi’s picture

dhruveshdtripathi commented

+1 for RTBC

kiwimind’s picture

kiwimind commented
Status: Reviewed & tested by the community » Needs work

Awesome, thanks for the start on this.

I've spotted a couple of small things that could be altered to help improve this. Quite happy to be pushed back on though.

  1. +++ b/README.txt
@@ -1 +1,98 @@
+It has been written to be fully extendable using drupal 8 APIs.

    Seeing as it's a brand name (or trademark or something), Drupal should be uppercase. 

  2. +++ b/README.txt
@@ -1 +1,98 @@
+    8. Select to region for block display from the Region dropdown.

    On the module's documentation, it mentions to perhaps only display this block on paths

    /private_messages
/private_messages/*

    Worth mentioning here?

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Assigned: Unassigned » volkswagenchick

Thanks for the review - I will work on this tomorrow.

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Assigned: volkswagenchick » Unassigned
volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2911812-2-8.txt1.44 KB
new private_message-add-useful-readme-2911812-8.patch3.72 KB

Thanks for the suggestions. I have uploaded a patch with the changes.

dani3lr0se’s picture

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

The additions look fine to me. I don't have anything else to add at the moment. I'm sure others may want to weigh in though. I'll mark RTBC in the meantime. Thanks @volkswagenchick!

kiwimind’s picture

kiwimind commented

Nice one, thanks for the patch. All looks good to me.

Happy to second that RTBC.

edutrul’s picture

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

Awesome job volkswagenchick, daniel_rose, kiwimind, dhruveshdtripathi
changes pushed! status as fixed!

Status: Fixed » Closed (fixed)

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