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
#7 stage_file_proxy-add_readme-2849596-7.patch2.1 KBmark_fullmer
#4 stage_file_proxy-add-readme-2849596-4.patch3.31 KBvolkswagenchick
#4 interdiff-2849596-2-4.txt1.18 KBvolkswagenchick
#2 stage_file_proxy-add-readme-2849596-2.patch3.31 KBvolkswagenchick
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

volkswagenchick created an issue. See original summary.

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
CreditAttribution: volkswagenchick at Hook 42 commented
Assigned: volkswagenchick » Unassigned
Status: Needs work » Needs review
FileSize
stage_file_proxy-add-readme-2849596-2.patch3.31 KB

Here is a useful README for the d8 branch. Thanks for this VERY USEFUL module.

loopduplicate’s picture

loopduplicate
she/her
Location Burning Token, California
CreditAttribution: loopduplicate at Hook 42 commented
Status: Needs review » Needs work

Looks good except for:

  1. +++ b/README.txt
@@ -0,0 +1,72 @@
+local machine. It will just serve a 301 (permanent URL redirection) and create a link to the original file on the remote server. ¶

    special character at the end of the sentence needs to be removed. 

  2. +++ b/README.txt
@@ -0,0 +1,72 @@
+By default, this variable is set to TRUE.  ¶

    Special character and extra space at the end of this line should be removed.

Regards,
Jeff

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
CreditAttribution: volkswagenchick at Hook 42 commented
Status: Needs work » Needs review
FileSize
interdiff-2849596-2-4.txt1.18 KB
stage_file_proxy-add-readme-2849596-4.patch3.31 KB

Thanks Loop. Here is a new patch, along with an interdiff, with the extra spaces removed. THANKS again!

loopduplicate’s picture

loopduplicate
she/her
Location Burning Token, California
CreditAttribution: loopduplicate at Hook 42 commented
Status: Needs review » Reviewed & tested by the community

Looks good :)

mark_fullmer’s picture

mark_fullmer
he/him
Primary language English
Location Tucson
CreditAttribution: mark_fullmer as a volunteer and at University of Texas at Austin commented
Status: Reviewed & tested by the community » Needs work

Drupal 8 doesn't support variable_set and $conf variables as described here. Apologies if I'm missing something, but this Readme seems applicable to 7.x, not 8.x.

I might encourage the OP to do a bit more diligence when creating READMEs. Quality over quantity! https://www.drupal.org/user/3332522/track/code

mark_fullmer’s picture

mark_fullmer
he/him
Primary language English
Location Tucson
CreditAttribution: mark_fullmer as a volunteer and at University of Texas at Austin commented
Status: Needs work » Needs review
FileSize
stage_file_proxy-add_readme-2849596-7.patch2.1 KB

The 8.x configuration is already thoroughly documented by the maintainers in the existing INSTALL.txt file. So, per module documentation guidelines, I feel it can remain there and the README can provide the "basic overview of what the module does and how someone may use it".

The patch attached does that, and moves the INSTALL.txt file to a INSTALL.md file.

loopduplicate’s picture

loopduplicate
she/her
Location Burning Token, California
CreditAttribution: loopduplicate at Hook 42 commented

Sounds good to me :) I wonder what you all think about wrapping consistency? Here's my thoughts:

  1. +++ b/README.md
@@ -0,0 +1,58 @@
+The Stage File Proxy module saves you time and disk space by sending requests to your development environment's files directory to the production environment and making a copy of the production file in your development site. It makes it

    Some lines break at 80 characters and some don't. Probably best to keep it consistent. Since it's not code, it's ok to extend beyond the 80 character mark but it seems fine to restrict all lines too. 

  2. +++ b/README.md
@@ -0,0 +1,58 @@
+Install the Stage File Proxy module as you would normally install a contributed Drupal module. Visit https://www.drupal.org/node/1897420 for more information.

    Same as above, this line goes beyond the 80 char mark whereas other lines are wrapped at 80 chars. 

  3. +++ b/README.md
@@ -0,0 +1,58 @@
+ to configure this within your settings.php or settings.local.php file. Detailed
+ descriptions of each setting, and syntax for defining the configuration in code
+ is in INSTALL.md

    When wrapping text, there shouldn't be a space before the wrapped lines.

Regards,
Jeff

BarisW’s picture

BarisW CreditAttribution: BarisW at LimoenGroen commented
Status: Needs review » Reviewed & tested by the community

Thanks all! I fixed the wrapping issues when committing it.

  • BarisW committed 3378d10 on 8.x-1.x authored by mark_fullmer 
    Issue #2849596 by volkswagenchick, mark_fullmer, loopduplicate, BarisW:...
BarisW’s picture

BarisW CreditAttribution: BarisW at LimoenGroen commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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