When evaluating this module I noticed that the README could use some useful configurations.
https://www.drupal.org/docs/develop/documenting-your-project/module-documentation-guidelines
Patch to follow, thanks!

CommentFileSizeAuthor
#7 simple_zoom-d8-readme-fixes-2943038-7.patch.txt2.3 KBloopduplicate
#7 interdiff-2943038-5-7.txt471 bytesloopduplicate
#5 simple_zoom-d8-readme-fixes-2943038-5.patch2.6 KBvolkswagenchick
#2 simple_zoom-d8-readme-fix-2943038-2.patch2.29 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 simple_zoom-d8-readme-fix-2943038-2.patch2.29 KB

I uploaded a patch that adds some useful configs to the README and the formatting aligns more to Drupal.org documentation standards

jqtnguyen’s picture

jqtnguyen commented
Status: Needs review » Needs work

Attempted to apply the patch on simplytest.me, but received an error message.

Also, found this:

+++ b/README.txt
@@ -1,25 +1,62 @@
+creates an image formatter and a CKEditor plugin for opening the image fields and

The line above exceeds 80 characters.

volkswagenchick’s picture

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

Thanks for the review. I am going to re-assign myself to the issue and patch again tomorrow. Sumplytest.me appears to be broken, hopefully that will be remedied by then.

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 simple_zoom-d8-readme-fixes-2943038-5.patch2.6 KB

I uploaded a patch that takes care of the long line. Thanks for the review!

jqtnguyen’s picture

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

I was able to apply the patch via simplytest.me. I was able to follow the instructions to enable and configure both Simple Zoom and Zoom for inline images.

I have one small suggestion for consistency that you might want to consider, but outside of that, everything looks really good.

+++ b/README.txt
@@ -1,25 +1,62 @@
+ * Enable the zoom_inline module. This automatically opens the inline images,

Change "zoom_inline" to "Zoom for inline images module" for consistency

loopduplicate’s picture

loopduplicate commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new interdiff-2943038-5-7.txt471 bytes
new simple_zoom-d8-readme-fixes-2943038-7.patch.txt2.3 KB 
+++ b/README.txt
@@ -1,25 +1,62 @@
+ * Navigate Administration > Structure > Content types > Content Type > Manage

"Navigate Administration" should be "Navigate to Administration"

Looks good other than that.

loopduplicate’s picture

loopduplicate commented

Regarding #6 "Change "zoom_inline" to "Zoom for inline images module" for consistency". I think we should leave the grammar the same as before and just address the module documentation guidelines. Perhaps another issue could be made to update the README for grammar and such and make the corresponding changes to the project page.

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Status: Needs review » Needs work

Thanks for the review and the new patch.

One minor nitpick.

+++ b/README.txt
@@ -1,25 +1,62 @@
+ * Navigate to Administration > Structure > Content types > Content Type > Manage

line length exceeds 80 characters

Marking needs work and assigning to myself. I will patch tomorrow.

  • sravya_12 committed 9c13a24 on 8.x-1.x 
    Issue #2943038 by volkswagenchick, loopduplicate: README configs and...
volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented

@sravya_12 - was this committed? Should the issue status be changed?
Thanks

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented

Thankyou for all the work you do!!
I am encouraging team mates to help out on Drupal.org, could you go back and give loopduplicate and JQTNguyen a credit for thier RTBCs?? The box next to their name just gets ticked and they get a credit! I see it as a way of incentive to help get involved it the community! It takes a village.

Cheers!

AmyJune - volkswagenchick

sravya_12’s picture

sravya_12 commented

Thank you all for the work !!
@volkswagenchick, Yes I totally agree with you to appreciate the contributors. The patch has been updated and the issue has not been closed as per your last comment of the patch to be updated. Closing the ticket now.

sravya_12’s picture

sravya_12 commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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