There is pretty widespread support for throwing custom modules in a /custom folder and contrib in a /contrib folder and it would be nice to point new users in the right direction.

Files: 
CommentFileSizeAuthor
#116 0001-Improving-README.txt-files-in-D7.patch7.02 KBdarol100
#110 0001-1539940-Encourage-best-practices-in-sites-README.txt.patch6.38 KBdarol100
PASSED: [[SimpleTest]]: [MySQL] 41,480 pass(es).
[ View ]
#106 1539940-Improving-the-README.txt-for-sites-all-modul.patch6.64 KBdarol100
PASSED: [[SimpleTest]]: [MySQL] 41,478 pass(es).
[ View ]
#89 sitesreadme-1539940-89-D7.patch6.22 KBjwilson3
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch sitesreadme-1539940-89-D7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#89 sitesreadme-1539940-84-89.interdiff.txt1.47 KBjwilson3
#85 sitesreadme-1539940-84-D7.patch6.24 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 39,469 pass(es).
[ View ]
#82 sitesreadme-1539940-81.patch6.43 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,897 pass(es).
[ View ]
#82 sitesreadme-1539940-79-81.interdiff.txt3.81 KBjwilson3
#80 sitesreadme-1539940-79.patch6.53 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,696 pass(es).
[ View ]
#80 sitesreadme-1539940-76-79.interdiff.txt3.97 KBjwilson3
#77 sitesreadme-1539940-76.patch6.63 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,329 pass(es).
[ View ]
#77 sitesreadme-1539940-69-76.interdiff.txt401 bytesjwilson3
#70 sitesreadme-1539940-69.patch6.36 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,329 pass(es).
[ View ]
#70 sitesreadme-1539940-66-69.interdiff.txt3.14 KBjwilson3
#67 sitesreadme-1539940-66.patch5.04 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,324 pass(es).
[ View ]
#67 sitesreadme-1539940-64-66.interdiff.txt1.01 KBjwilson3
#65 sitesreadme-1539940-64.patch5.12 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,324 pass(es).
[ View ]
#65 sitesreadme-1539940-56-64.interdiff.txt3.35 KBjwilson3
#61 sitesreadme-1539940-56.patch4.79 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,322 pass(es).
[ View ]
#61 sitesreadme-1539940-48-56.interdiff.txt249 bytesjwilson3
#48 sitesreadme-1539940-48.patch4.79 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,297 pass(es).
[ View ]
#48 sitesreadme-1539940-46-48.interdiff.txt137 bytesjwilson3
#46 sitesreadme-1539940-46.patch4.78 KBjwilson3
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch sitesreadme-1539940-46.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#46 sitesreadme-1539940-45-46.interdiff.txt431 bytesjwilson3
#45 sitesreadme-1539940-45.patch4.39 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,295 pass(es).
[ View ]
#45 sitesreadme-1539940-42-45.interdiff.txt404 bytesjwilson3
#44 sitesreadme-1539940-42.patch4.39 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,311 pass(es).
[ View ]
#44 sitesreadme-1539940-40-42.interdiff.txt2.64 KBjwilson3
#40 sitesreadme-1539940-40.patch4.11 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,301 pass(es).
[ View ]
#40 sitesreadme-1539940-39-40.inter_.diff365 bytesjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,313 pass(es).
[ View ]
#40 sitesreadme-1539940-38-40.inter_.diff1.92 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,299 pass(es).
[ View ]
#40 sitesreadme-1539940-36-40.inter_.diff2.42 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,307 pass(es).
[ View ]
#39 sitesreadme-1539940-39.patch4.1 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,299 pass(es).
[ View ]
#39 sitesreadme-1539940-36-39.inter_.diff2.41 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,307 pass(es).
[ View ]
#39 sitesreadme-1539940-38-39.inter_.diff1.91 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,306 pass(es).
[ View ]
#38 sitesreadme-1539940-38.patch4.05 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,311 pass(es).
[ View ]
#37 sitesreadme-1539940-36.patch3.86 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,293 pass(es).
[ View ]
#35 sitesreadme-1539940-28.patch3.49 KBjwilson3
PASSED: [[SimpleTest]]: [MySQL] 41,297 pass(es).
[ View ]
#32 README.txt214 bytesNiklas Fiekas
#27 sitesreadme-1539940-27.patch2.33 KBinfiniteluke
PASSED: [[SimpleTest]]: [MySQL] 35,411 pass(es).
[ View ]
#24 sitesreadme-1539940-24.patch2.34 KBinfiniteluke
PASSED: [[SimpleTest]]: [MySQL] 35,432 pass(es).
[ View ]
#20 sitesreadme-1539940-20.patch1.23 KBinfiniteluke
PASSED: [[SimpleTest]]: [MySQL] 35,352 pass(es).
[ View ]
#18 sitesreadme-1539940-18.patch1.25 KBinfiniteluke
PASSED: [[SimpleTest]]: [MySQL] 35,361 pass(es).
[ View ]
#17 sitesreadme-1539940-17.patch1.25 KBinfiniteluke
PASSED: [[SimpleTest]]: [MySQL] 35,366 pass(es).
[ View ]
#13 sitesallmodulesreadme-1539940-13.patch437 bytesinfiniteluke
PASSED: [[SimpleTest]]: [MySQL] 35,364 pass(es).
[ View ]
#8 sitesallmodulesreadme-1539940-8.patch476 bytesinfiniteluke
PASSED: [[SimpleTest]]: [MySQL] 35,362 pass(es).
[ View ]
modules-folder-readme.patch572 bytesmrf
PASSED: [[SimpleTest]]: [MySQL] 35,353 pass(es).
[ View ]

Comments

jhodgdon’s picture

Hmmm... I am not sure this has "widespread support", although someone added it as a possible suggestion for sites with lots of modules to
http://drupal.org/documentation/install/modules-themes/modules-7

But for instance, the Drupal 7/8 core module that lets you install modules doesn't do this. My feeling is that we probably shouldn't make it a recommendation, but I'm open to being swayed by public opinion.

In any case, the patch also has some grammatical/punctuation errors that would need to be fixed before it could be committed, and I'd like to see several people weigh in on whether we want to recommend this before considering it too. So in spite of the grammar/punctuation problems, I'm leaving it at "needs review" to see if anyone else has an opinion.

chrisparsons’s picture

Just as another data point, we use a /custom folder for our custom modules, but contrib modules just go within sites/all/modules. Drush likes to put things within sites/all/modules by default as well, if my memory of the default settings for Drush is not foggy.

I'm not necessarily providing opposition to this change -- if D8 is going this route, it'd be nice to go one step further, and include those two folders in the directory structure and not just reference them in readme.txt, but I'd have to agree with jhodgdon that it doesn't necessarily have widespread support.

tim.plunkett’s picture

I know that I hate when I inherit sites that have this structure. I don't think that this personal preference should be codified in core like this.

jhodgdon’s picture

Here's a thought. We could patch the README file text in some way that makes it clear that you are free to organize your modules into subdirectories within sites/all/modules if you want to (i.e., that Drupal will look there and in subdirectories to find them). That would be a neutral way to introduce the idea, without promoting it. Exact wording TBD, but would that make sense?

jhodgdon’s picture

Status:Needs review» Needs work

In any case, it doesn't seem like there's enough support to change the wording as suggested. Anyone interested in a patch along the lines of #5?

webchick’s picture

Issue tags:+Novice

#5 sounds good, and would be a novice task.

infiniteluke’s picture

Status:Needs work» Needs review
StatusFileSize
new476 bytes
PASSED: [[SimpleTest]]: [MySQL] 35,362 pass(es).
[ View ]

Here's a stab at the wording. I kept it simple without any examples, as I felt adding examples would imply promotion of the examples.

This directory should be used to place downloaded and custom modules
which are common to all sites. It is safe to organize modules into
sub-directories. This will allow you to more easily
update Drupal core files.

mrf’s picture

Guess I could have been clearer about what I meant by widespread support. I don't see a clear pattern in use everywhere, but drush and other tools allow you to specify sub folders and even auto detect certain common names. I think just pointing out the possibility of using subfolders is enough without recommending a particular style.

hefox’s picture

re drush: Drush will put downloaded contrib modules into contrib on drush dl if it exists to my memory.

Berdir’s picture

Placing custom modules in sites/site/modules stops making sense the second you are using multisite and want to share custom modules between them.

While maybe not a standard, I think custom/contrib folders are a fairly standard pattern and is supported by drush out of the box: if you have a contrib folder, drush dl will automatically put them in that folder.

jhodgdon’s picture

Status:Needs review» Needs work

The way this (#8) proposed paragraph is, it sounds to me like it is saying that using subdirectories, will allow you to update core easier. That isn't the case -- using sites/all/modules will allow you to update core easier. So it needs to be rearranged.

Re #11, maybe we should also point out that you can/should use sites/[site]/modules for modules that are specific to one particular site, if you are using multi-site, or maybe say that sites/all/modules is for modules that are common to all sites if you are using multi-site?

infiniteluke’s picture

Status:Needs work» Needs review
StatusFileSize
new437 bytes
PASSED: [[SimpleTest]]: [MySQL] 35,364 pass(es).
[ View ]

Re #8: I agree that the wording was confusing. I moved the sentence to the end.

Re #12: I left out any mention of the sites directory because this readme is specific to the sites/all/modules directory. It seems like a separate readme in the sites directory would be more appropriate to explain the multi-site setup (sites/[site]/modules).

Agree/Disagree?

jhodgdon’s picture

There is currently not a README in the sites directory at all, so sites/all is all we have... but I guess it does say "that are common to all sites", so that is probably good enough.

Hmmm.

Actually, we currently have README files in sites/all as well as sites/all/modules and sites/all/themes. Should they all be updated, or should we maybe put the new text into the one in sites/all and change the other two to say "See README up one directory" and/or remove those other ones?

The current text is fine...

infiniteluke’s picture

Is it not an option to add a README to the sites directory? I think this would be the best place to address the multi-site option since it is the directory that sites are added.

I agree that the sites/all/themes README should include the same sentence. Here is a separate issue with a patch: #1543724: Update sites/all/themes/README.txt to mention sub-directories

I think the sites/all README is fine as-is unless that's where we decide to put the reference to multi-sites.

jhodgdon’s picture

Let's just do all the README's here on this one issue. And yes, I think we can add one to the sites directory.

infiniteluke’s picture

StatusFileSize
new1.25 KB
PASSED: [[SimpleTest]]: [MySQL] 35,366 pass(es).
[ View ]
  • Updated sites/all/themes README to include "It is safe to organize modules into sub-directories."
  • Includes prior change to sites/all/modules README in #13.
  • Added README to sites directory with following text:

This directory can be used for a multi-site setup. Multi-site allows you to
share a single Drupal installation (including core code, contributed
modules, and themes) among several sites. For more information see
http://drupal.org/documentation/install/multi-site

infiniteluke’s picture

StatusFileSize
new1.25 KB
PASSED: [[SimpleTest]]: [MySQL] 35,361 pass(es).
[ View ]

Fixed sites/all/themes README to read "It is safe to organize themes into sub-directories."

jhodgdon’s picture

Status:Needs review» Needs work

RE #17 -- looks pretty good! The only thing I think I would change is the sites/README... How about instead of "core code, contributed modules, and themes", we say something like "core code, as well as contributed and custom modules and themes"? And rather than pointing to that drupal.org page, we could point to the core/INSTALL.txt file, which has information on how to do multi-site installations (including where to put modules/themes).

infiniteluke’s picture

StatusFileSize
new1.23 KB
PASSED: [[SimpleTest]]: [MySQL] 35,352 pass(es).
[ View ]

Reworded as mentioned in #19.

infiniteluke’s picture

Status:Needs work» Needs review
Niklas Fiekas’s picture

+++ b/sites/all/modules/README.txtundefined
@@ -1,4 +1,5 @@
+update Drupal core files. It is safe to organize modules into
+sub-directories.

Just a note: Why are we breaking the line before 80 characters? I see that in the context, too - not sure why.

jhodgdon’s picture

Status:Needs review» Needs work

Good point, it looks like that line can/should include sub-directories without going over 80 characters. The other lines should be checked for that.

infiniteluke’s picture

Status:Needs work» Needs review
StatusFileSize
new2.34 KB
PASSED: [[SimpleTest]]: [MySQL] 35,432 pass(es).
[ View ]

Line breaking only when over 80 characters for all four files

sites/all/modules/README.txt
sites/all/themes/README.txt
sites/all/README.txt
sites/README.txt

ryanissamson’s picture

Status:Needs review» Reviewed & tested by the community

Lines don't go over 80.

jhodgdon’s picture

Status:Reviewed & tested by the community» Needs work

There is a minor grammatical error in this patch:

+This directory should be used to place downloaded and custom modules and themes
+which are common to all sites. Keeping contributed and custom modules and

which -> that

This also applies to several other spots in the patch.

See http://www.english-the-easy-way.com/Determiners/That_Which.htm and also note that if you use "which", it should nearly always have a comma before it.

I'm also wondering if this sentence:

Keeping contributed and custom modules and themes in the sites directory will aid in upgrading Drupal core files.

should be in the sites rather than sites/all README, since it applies to sites/[specific site] as well? I think it should.

infiniteluke’s picture

Status:Needs work» Needs review
StatusFileSize
new2.33 KB
PASSED: [[SimpleTest]]: [MySQL] 35,411 pass(es).
[ View ]

I copy and pasted that sentence. That'll teach me :)

I corrected the use of "which" in the other files as well.

Also made white space consistent across files.

jwilson3’s picture

Status:Needs review» Needs work

I'm not sure I'm fond of placing the statement about multi-site *before* the more important statement about what the sites folder structure actually represents. The way its written, I could imagine a novice reading this-- scratching their head-- thinking, "hrm, this feature is for multiple sites, and i only have a single site".

The order should be:

a) sentence about what the sites folder is / does in the most general sense. (ie, it holds all of the non-core code used to build your particular website(s)).
b) sentence about why that is a Good Thing. (ie, this makes site maintenance easier and enables clean core updates).
c) alternative or advanced usage. ie (folder is structured in a way that it can also be used in a multi-site configuration).
d) where to look for more info (ie, link to INSTALL.txt)

jwilson3’s picture

Another point:

While I understand the purpose of "It is safe to organize (modules|themes) into sub-directories." This seems out of left field, when you read this file as a novice. Without an example to follow, or a place to read about possible common organizational configurations, this sentence leaves you with more questions than answers. I also fear that novices would get overzealous and start installing modules into groups of folders based on functionality, or better yet, installing modules inside modules.

Inheriting a site that has no VCS, but has a logical structure, eg a folder for patched, custom, contrib, and features, is far better than inheriting one with seemingly random sites/all/modules structure based on grouped functionality, or families of modules.

jhodgdon’s picture

RE #29 - I don't think we want to expand the README that much to add in examples, but I would be comfortable with adding something like, ", such as contributed, custom, and patched" to that sentence (which I'm sure the original reporter of this issue would agree with. :)

RE #28 - that sounds like a good suggestion.

rsgracey’s picture

So what does the final text look like?--it's hard to read all the patch stuff... Since it's just a README.txt file, would someone please post it?

Niklas Fiekas’s picture

StatusFileSize
new214 bytes

Sure. This would be it as of the patch from #27. The README for themes is almost the same. The newly added README is the first hunk in the patch - additions only, so that should be readable.

ronan.orb’s picture

I find the term "sub-directories" missleading because every module in Drupal is a collection of sub-direcories already. By installing 6 modules I already have the feeling of "organising" them in sub-directiores. Why not suggesting a good practice like modules/all/contrib for everything from drupal.org and modules/all/custom for own stuff.

It would be still a good practice suggestion. I sense to much fear here.

I have at least two projects with over 50 modules were 1/3 is my own. I would like to reorganise them now, but I fear the reordering. Not sure if a simple "patch" to the system table is all I need. If back in time there would be a suggestion which sub-directories to use for the various sources of modules I would now don't have that mess.

Berdir’s picture

Just move the modules around as like and then clear the caches, Drupal will find them again and update the system table. There are some exceptions, for example with modules that define classes which have an absolute path to the file and are loaded early (mostly entity controllers). You can either fix that yourself in the registry table or use a tool like Registry Rebuild.

jwilson3’s picture

Status:Needs work» Needs review
StatusFileSize
new3.49 KB
PASSED: [[SimpleTest]]: [MySQL] 41,297 pass(es).
[ View ]

Per http://drupal.org/node/1724216 and http://drupal.org/node/1766160, this re-roll takes into account the latest changes from head, namely that the files have moved.

I've made some additional effort to document the profiles folder, since it is not documented very well at all elsewhere.

jhodgdon’s picture

Status:Needs review» Needs work

Wow, I did not know about those changes! Thanks for updating the patch!

A few thoughts and things to fix:

a) The change notice http://drupal.org/node/1766160 says that /sites/all/modules takes precedence over /modules (and presumably the same for profiles/themes), so I think that should be mentioned in the READMEs.

b) This sentence in the modules readme:

+Refer to the DEVELOPING FOR DRUPAL section of the README.txt in the Drupal root...

Inside of that README, the section is referred to as "Developing for Drupal" (also referred that way in the table of contents at the top). So I think we can drop the ALL CAPS here. It's very shouty. :)
==> The same thing applies to the themes readme.

c) This sentence in the profiles readme:

+In multisite configuration, installation profiles found in this directory are
+available to all sites during their initial site installation, however modules
+and themes located inside a given profile are only available to sites that
+installed with that specific profile. In addition to this directory, common

- punctuation: should be "...initial site installation; however, modules..."
- installed is a transitive verb. The last line should be "were installed with...".

d) The end of the profiles readme refers to themes instead of profiles.

+should be used to restrict themes to specific site instances.

e) Formatting in the profiles readme near the end:

+Read about the difference between installation profiles and distributions:
+
+* http://drupal.org/node/1089736
+
+Community-contributed installation profiles and distributions may be downloaded
+freely from Drupal.org at:
+
+* http://drupal.org/project/distributions

We don't normally use * to indicate bullet lists in Drupal docs. We use - instead, and there shouldn't be a blank line between the : and the start of the list. But we also don't normally have lists of one item. I think these should both be turned into ordinary sentences, such as:

Read about the difference between installation profiles and distributions at http://drupal.org/node/1089736.

f) Technically, sites/all/profiles did not previously work, so I think the statement about "compatibility" when talking about sites/all/profiles should be removed from the profiles readme.

g) The previous patches on this issue recommended another practice, which was making sub-directories for "contrib" and "custom" and stuff like that. This patch does not have those changes. Take a look at the previous patches and make sure all the wording in there that was being worked on gets into the next patch please. (See patch in #27 and the next three comments.)

jwilson3’s picture

StatusFileSize
new3.86 KB
PASSED: [[SimpleTest]]: [MySQL] 41,293 pass(es).
[ View ]

Excellent review jhodgdon!

a) Fixed. Added a statement about precedence in all three READMEs. However, as this is a truly edge case scenario (precedence only matters when the module/theme/profile is named exactly the same) I question whether having this bit is very useful in the readme, or should be relegated off to just a comment in the code. Thoughts?

b) Fixed. Replaced ALL CAPS references with "Title Case" references in quotations, to match /README.txt (in Drupal root).

c) I split the "; however, modules and themes located inside a give profile" sentence up and moved it to the end of the paragraph for better flow / readability.

d) Fixed.

e) Fixed. I adapted the further reading section for profiles to the format used in the modules and themes sections of /README.txt (in Drupal root). I wonder if this more detailed blurb in /profiles/README.txt should now rather be moved to /README.txt (for consistency with modules and themes).

f) I change the statement about allowing sites/all/profiles from "for compatibility" to "for consistency", which does make more sense.

g) Unless I missed something, the brunt of the change from #27 about sub-directories was just to add the text "It is safe to organize themes into sub-directories." to the modules and themes README.txt. I did not add this text to the profiles section, but did add a statement about adding modules/themes *inside* profiles. Please review.

h) NEW ADDITION: I forgot to *add* /sites/README.txt, which was introduced in previous patches. This is remedied.

i) ADDITIONAL CHANGE: I changed references from sites/[instance]/modules to sites/your_site_name/modules (for consistency with core). There is one place in core where this is referred to as your_site_dir, which should be changed too ;)

j) QUESTION: I'm unsure about consistency with leading slashes now on sites/all/[X] and sites/your_site_name/[X]. /README.txt does use references to /modules and /themes folders, but other places in the code still refer to sites/all and sites/default and sites/your_site_name without the leading slash. Topic for a separate discussion I suppose.

jwilson3’s picture

Title:Update sites/all/modules/README.txt to recommend sub folder best practices» Encourage best practices in /sites/README.txt, /modules/README.txt, /profiles/README.txt and /themes/README.txt
Status:Needs work» Needs review
StatusFileSize
new4.05 KB
PASSED: [[SimpleTest]]: [MySQL] 41,311 pass(es).
[ View ]

This issue has blossomed into an entire overhaul of all the README.txts, so I thought I'd update the title appropriately.

Inline with the new title, IMHO it makes sense to mention the best practice of using Drupal's sub-theme functionality in /themes/README.txt to ease maintenance and upgrades. Even though it is a slightly advanced topic, its still considered a best practice! I suppose this is open to debate and input from more senior Drupal contributors, but I've added it anyway. ;)

Also fixed a couple minor issues:

< +kept in the sites/all/modules directory for historic and compatibility reasons;
---
> +kept in the sites/all/modules directory for historic and compatibility reasons.
< +directory for further information on theming.s
---
> +directory for further information on theming.

Finally, I've added a link to developing distributions, to match with similar links to themes/modules. (I still think the detailed blurb in /profiles/README.txt could be moved to /README.txt).

> +* Develop your own install profile or distribution:
> +  http://drupal.org/developing/distributions
jwilson3’s picture

StatusFileSize
new1.91 KB
PASSED: [[SimpleTest]]: [MySQL] 41,306 pass(es).
[ View ]
new2.41 KB
PASSED: [[SimpleTest]]: [MySQL] 41,307 pass(es).
[ View ]
new4.1 KB
PASSED: [[SimpleTest]]: [MySQL] 41,299 pass(es).
[ View ]

A couple more changes (sorry for jumping the gun).

I was not pleased with the difference sentence structures in the multisite instructional blurbs between modules/themes/profiles.

This patch brings better sentence consistency to those paragraphs.

Also an interdiff of the changes against #38, and #36 to make it easier to see what's changed.

jwilson3’s picture

StatusFileSize
new2.42 KB
PASSED: [[SimpleTest]]: [MySQL] 41,307 pass(es).
[ View ]
new1.92 KB
PASSED: [[SimpleTest]]: [MySQL] 41,299 pass(es).
[ View ]
new365 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,313 pass(es).
[ View ]
new4.11 KB
PASSED: [[SimpleTest]]: [MySQL] 41,301 pass(es).
[ View ]

OMG I missed an "Alternatively"... #facepalm I think its ready now..

jwilson3’s picture

In regards to the original request to indicate best practices of 'contrib', 'custom' (and maybe 'patched') sub-folders... I wrote up this text, but later 'nixed it.

It is safe to organize modules into sub-directories to help identify where
modules come from and to ease maintenance and apply security updates. One
prevalent organization technique places clean versions of contributed modules
inside a 'contrib' sub-directory and places your in-house custom-developed
modules inside a 'custom' sub-directory.  Some developers then also move
hacked or patched modules from the contrib space into a 'patched' sub-diretory
to further help them remember to reapply patches when updating those modules.
Note that moving modules into subfolders after initial installation may require
the Drupal registry to be rebuilt.

Adding it here, if anyone thinks its worthy of inclusion to elaborate the best practices. Maybe this could become the basis for inclusion in best practices documentation online that can then be referenced to explain what we mean by "organize modules into sub-directories".

ryanissamson’s picture

Status:Needs review» Needs work

A few suggestions and fixes here.

+++ b/modules/README.txtundefined
@@ -1,4 +1,14 @@
+This directory should be used to keep downloaded and custom modules that extend
+your site's functionality separate from Drupal core, encouraging best practices
+and facilitating safer self-contained code updates. It is safe to organize

maybe...

"This directory should be used to keep downloaded and custom modules that extend your site's functionality beyond Drupal core. This encourages best practices and facilitates safe, self-contained code updates."

+++ b/modules/README.txtundefined
@@ -1,4 +1,14 @@
+directory. Alternatively, the sites/your_site_name/modules directory pattern may

Is this meant to be "...pattern may be used to restrict modules..." instead of "restricted"?

+++ b/profiles/README.txtundefined
@@ -1,4 +1,30 @@
+defining content types, etc.) that run after Drupal's base installation when
+you first install Drupal and are what developers create as the basis of

I think this is the only place "you" is used, maybe take it out to make it consistent throughout?

"...when Drupal is first installed and are what developers create as the basis of distributions."

+++ b/profiles/README.txtundefined
@@ -1,4 +1,30 @@
+directory pattern may be used to restrict profiles availability to a specific

missing article

"...used to restrict a profile's availability..."

+++ b/profiles/README.txtundefined
@@ -1,4 +1,30 @@
+Additionally, modules and themes may be placed inside sub-directories in a

I don't think we need a dash in subdirectories.

+++ b/themes/README.txtundefined
@@ -1,4 +1,15 @@
+This directory should be used to keep downloaded and custom themes that improve
+the appearance of your site separate from Drupal core themes, encouraging best
+practices and facilitating safer self-contained code updates. It is safe to
+organize themes into sub-directories and is recommended to use Drupal's

Same minor change in wording and change to subdirectories.

"This directory should be used to keep downloaded and custom themes that alter the appearance of your site beyond Drupal's core themes. This encourages best practices and facilitates safer self-contained code updates. It is safe to organize themes into subdirectories..."

jhodgdon’s picture

Great work, keep it up... Just a note that when you upload an interdiff, use a .txt extension so it doesn't get run through the tests. :)

And yes I think we should say something about the possibility of organizing your modules into subdirectories within modules or sites/all/modules [same with themes & profiles] -- but maybe not quite as lengthy as #41. How about just something like:

It is safe to organize modules into sub-directories, such as "contrib" for contributed modules downloaded from Drupal.org, and "custom" for custom modules. Note that if you move a module to a subdirectory after it has been enabled, you may need to clear the Drupal cache so that it can be found.

(I'm not sure that this 2nd sentence is necessary though -- maybe someone can do a quick test in both D8 and D7 to see what happens, hopefully with a module that includes some PSR-0 classes, or comment here with the expected behavior? I'm not sure clearing the cache is even the right thing to do... but I think so?)

jwilson3’s picture

Status:Needs work» Needs review
StatusFileSize
new2.64 KB
new4.39 KB
PASSED: [[SimpleTest]]: [MySQL] 41,311 pass(es).
[ View ]

Incorporated changes from #42 and #43, but we still need someone's input about moving modules around after they've been enabled -- I based the original statement in #41 (about registry rebuild), on info from #34.

jwilson3’s picture

StatusFileSize
new404 bytes
new4.39 KB
PASSED: [[SimpleTest]]: [MySQL] 41,295 pass(es).
[ View ]

Column wrapping was off on one of the lines that was changed...

jwilson3’s picture

StatusFileSize
new431 bytes
new4.78 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch sitesreadme-1539940-46.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Hrm, just noticed sites/README.txt is not getting added to my diffs...

Just learned that adding the file with git add -N makes it show up in git diff.

Status:Needs review» Needs work

The last submitted patch, sitesreadme-1539940-46.patch, failed testing.

jwilson3’s picture

Status:Needs work» Needs review
StatusFileSize
new137 bytes
new4.79 KB
PASSED: [[SimpleTest]]: [MySQL] 41,297 pass(es).
[ View ]

Grr. and for the record.... git add -N *doesnt* work.

Well at least I'm learning the *right* way to make a patch...

Ended up creating a topic branch "1539940-readmes", committing all the files, rebasing against 8.x branch, and doing a git diff 8.x 1539940-readmes.

Niklas Fiekas’s picture

(Side note: Yes, that might be the better way, anyway. But what about git add -N doesn't work? Was the file affected by a .gitignore? Then git add -N -f should do it.)

jwilson3’s picture

@Niklas Feikas.... see the interdiff between #46 and #48.

Essentially, git add -N creates a patch file with a signature for *new files* that looks like:

diff --git a/sites/README.txt b/sites/README.txt
index e69de29..361370a 100644
--- a/sites/README.txt
+++ b/sites/README.txt

But this doesn't apply, it needs to use /dev/null

diff --git a/sites/README.txt b/sites/README.txt
new file mode 100644
index 0000000..361370a
--- /dev/null
+++ b/sites/README.txt

(/me shrugs)

Niklas Fiekas’s picture

Oh, that's indeed surprising. I considered asking the git mailing list. On the second glance it makes some sense, though. git add -N adds an empty blob (e69de29) to the tree in the index. Then git diff will diff that empty but existing file (a/sites/README.txt) from the index against the real file. Now all we need to fail is the extremly strict git apply, that notices that the file does not exist in the target repo. Other tools like patch -p1 aren't that strict.

Cut. Back to the actual issue :)

ryanissamson’s picture

Status:Needs review» Needs work

This is starting to look good.

+++ b/themes/README.txtundefined
@@ -1,4 +1,15 @@
+This directory should be used to keep downloaded and custom themes that improve
+the appearance of your site beyond Drupal core themes. This encourages best

My only suggestion is that "improve" might be a bit relative/misleading. Core themes can be perfect for some needs. Some who are new to Drupal might feel overly compelled to try contrib/custom themes even if they aren't ideal for the current situation.

I think "alter" or "customize" might be more accurate as contrib themes definitely do this, but I'm certainly open to being wrong here.

jwilson3’s picture

My only suggestion is that "improve" might be a bit relative/misleading. I think "alter" or "customize" might be more accurate as contrib themes definitely do this.

If you look at it from the end-user site-builder point of view for whom these readme's exist, installing both modules and themes from contrib *is* an improvement on their site, but I can understand that this could rub some folks the wrong way.

However, I hesitate to use the word "alter" because the vast majority of themes don't actually depend on core themes as their base, and using "alter" in that context actually muddies the water between a general contrib theme and the sub-theme methodology, which really is akin to altering or customizing a base theme.

Finally, "customize", in theory would work as a correct verb in this case, but its redundant: "... custom themes that customize...". Rewording the sentence to get rid of the redundancy would then make the first line vary even further between the three readmes.

Do you have further ideas for improving the way this carries over that would work between all three readmes in a consistent way?

jhodgdon’s picture

OK, the last patch had:

+This directory should be used to keep downloaded and custom themes that improve
+the appearance of your site beyond Drupal core themes. This encourages best

I think we should just say "Put themes that you download or create in this directory."

ryanissamson’s picture

Well, if we're trying to make this sounds consistent across the readmes we might want to stick with something closer to the other two. Also, I still think it's best to keep personal pronouns out of this as much as possible to keep it uniform.

What about using "change"? It's accurate and doesn't signify better or worse.

"This directory should be used to keep downloaded and custom themes that allow you to change the appearance of your site beyond Drupal core themes."

jwilson3’s picture

What about "modify"?

Eg,

"This directory should be used to keep downloaded and custom themes that modify the appearance of your site beyond Drupal core."

jhodgdon’s picture

change/modify both sound fine to me.

ryanissamson’s picture

Modify works for me!

jwilson3’s picture

re personal pronouns:

Earlier someone already asked me to remove all reference to "you", and #55 would reintroduce that...

IMHO "your site" is less direct than "you", but still personable enough to have weight with people that need to be heeding the ideas presented in these files.

jwilson3’s picture

re: "put themes that you download or create in this directory."

The original intent of this sentence was to explain the best practice of keeping contrib separate from core, which is why core is even mentioned in the first sentence. Originally the sentence was structured directly in this way: using the terminology "keep modules/themes/profiles ... separate from Drupal core.". This was later requested to be changed to "keep modules/themes/profiles that do X ... beyond Drupal core".

I'm open to a refactoring of this sentence to make it as short and to the point as possible, but without losing the original intent, which hints at the reasoning behind why to use this folder instead of the core modules/themes/profiles folder. Now, its arguable that since these have been shuffled off to the /core folder, that people will be less likely anyway to place stuff there, so its not necessary to explain this in detail. On the contrary, occasional drupal users who may have learned to previously *not* use modules & themes folder in drupal root need to be shown that they can now use these folders, and in fact, its a new recommended best practice.

jwilson3’s picture

StatusFileSize
new249 bytes
new4.79 KB
PASSED: [[SimpleTest]]: [MySQL] 41,322 pass(es).
[ View ]

Modify it is (for now anyway)!

jwilson3’s picture

Given #60, i wonder if it would make sense to go back to the original way, but change "that" to "which", and insert some commas to connect "keep separate" into one single idea.

Eg.

"This directory should be used to keep downloaded and custom modules, which extend your site's functionality, separate from Drupal core."

"This directory should be used to keep downloaded and custom themes, which modify the appearance of your site, separate from Drupal core themes."

jwilson3’s picture

Here's one final idea that counter's my last post in #62, and builds on short and sweet #54:

"Place downloaded and custom themes in this directory. This ensures clean separation from Drupal core and facilitates safe, self-contained code updates."

"Place downloaded and custom installation profiles (a.k.a. install profiles) in this directory. This ensures clean separation from Drupal core install profiles and facilitates safe, self-contained code updates."

"Place downloaded and custom modules in this directory. This ensures clean separation from Drupal core modules and facilitates safe, self-contained code updates."

UPDATE: Site note, the obvious thing that these proposed changes lack that the existing proposals have is the statement that answers "what is a module?" (something that extends your sites functionality) and "what is a theme?" (something that changes your site's appearance).

jwilson3’s picture

Here's a more complete thought with simple, straightforward sentences.

Themes modify the appearance of your Drupal site. Placing downloaded and custom
themes in this directory ensures clean separation from Drupal core and facilitates
safe, self-contained code updates. Community-contributed themes are available
for free download and usage at http://drupal.org/project/themes.

It is safe to organize themes into subdirectories and is recommended to use Drupal's
sub-theme functionality to ensure easy maintenance and upgrades.. [...]

Thoughts? Should I roll with this?

jwilson3’s picture

StatusFileSize
new3.35 KB
new5.12 KB
PASSED: [[SimpleTest]]: [MySQL] 41,324 pass(es).
[ View ]

So... I rolled with it... and, I'm sorry if the near-stream-of-consiousness posts here are throwing off people's reviews... I'm on a roll and had to get this thought out on the page. I hope that my interdiffs will be helpful to mitigate this a bit. I'd be happy to upload addition interdiffs from older versions if someone requests it.

This change rolls the first paragraphs of the three readmes up into something a bit more bite-size with simple sentence structure. The sentences aren't as short as the could be in that suggestion in #64, but i really feel these are more complete and well rounded. Check it out.

For those that dont want to read the patch format the files now read like this:

Place downloaded and custom modules that extend your site functionality beyond
Drupal core in this directory to ensure clean separation from core modules and
to facilitate safe, self-contained code updates. Contributed modules from the
Drupal community are freely available for download and usage at
http://drupal.org/project/modules.

It is safe to organize modules into subdirectories, such as "contrib" for
contributed modules, and "custom" for custom modules. Note that if you move a
module to a subdirectory after it has been enabled, you may need to clear the
Drupal cache so that it can be found.

Place downloaded and custom themes that modify your site's appearance in this
directory to ensure clean separation from Drupal core and to facilitate safe,
self-contained code updates. Contributed themes from the Drupal community are
freely available for download and usage at http://drupal.org/project/themes.

It is safe to organize themes into subdirectories and is recommended to use
Drupal's sub-theme functionality to ensure easy maintenance and upgrades.

Place downloaded and custom installation profiles in this directory to ensure
separation from Drupal core profiles and to facilitate safe, self-contained code
updates.

Installation profiles (a.k.a. install profiles) define additional installation
steps (such as enabling modules, defining content types, etc.) that run after
the base installation from core when Drupal is first installed and are what
developers create as the basis of distributions. Contributed install profiles
and distributions from the Drupal community are freely available for download
and usage at http://drupal.org/project/distributions.

For those with a super-keen eye, yes, the profiles readme is structured slightly differently than modules/themes readme. Namely, I deliberately left the answer to the question "what is an intall profile" out of the first sentence. I left it as a second paragraph because there is no way to explain it, and introduce distributions, as well as tell people to put them in this directory in one single sentence.

jhodgdon’s picture

Thanks, nice work! I have a couple of small comments; other than these two comments, I really like the patch in #65... Any other opinions?

a) I don't like this wording much:

Contributed modules from the
+Drupal community are freely available for download and usage at
+http://drupal.org/project/modules.

"freely available for download and usage" -- ?!? How about just "available for download".

b) Did anyone actually check on the part about:

Note that if you move a
+module to a subdirectory after it has been enabled, you may need to clear the
+Drupal cache so that it can be found.

?

jwilson3’s picture

StatusFileSize
new1.01 KB
new5.04 KB
PASSED: [[SimpleTest]]: [MySQL] 41,324 pass(es).
[ View ]

"freely available for download and usage" -- ?!?

What... you smell a marketing pitch? :P

Would this work instead: "Contributed [themes|modules|profiles] from the Drupal community may be downloaded at http://..."?

Re: moving a module after being enabled... In #43 you mention modules that have PSR-0 classes. Are we even at a point where there are Drupal 8 contrib modules that could be used to verify this? If not, could we verify with a core module? If so, which one(s)?

jwilson3’s picture

Status:Needs work» Needs review
jhodgdon’s picture

Status:Needs review» Needs work

I think this is looking good! I just have a couple of minor thoughts/concerns:

a) I think the "more about profiles" section belongs in the main Drupal README, similar to the information about themes and modules that is in the main README -- rather than putting it into profiles/README.txt... Maybe the sentences explaining what a profile is also belong there? In the modules/themes readmes we don't explain in detail what a module/theme is -- they just say "See the xyz section in the main readme for details".

b) Instead of a.k.a., write out "also known as" (this is in the profiles readme). Remember, we have many users of Drupal for whom English is not a completely familiar language, yet they may be using the English README.

c)

+See core/INSTALL.txt for information about single-site installation or
+multisite configuration.

See that we have "single-site" and "multisite"? This seems wrong to me... I think the latter should be "multi-site" maybe? We definitely wouldn't write "singlesite"...

jwilson3’s picture

StatusFileSize
new3.14 KB
new6.36 KB
PASSED: [[SimpleTest]]: [MySQL] 41,329 pass(es).
[ View ]

#69.a) Move profiles detail to main Drupal README.

I mentioned this idea in comment #37 item e, and again in #38, and this was the confirmation I was looking for. Glad you agree. I've worked up a first go at an "Installation Profiles" section in the main Drupal README.txt, but it could stand to be shortened up a bit -- it's a little wordy still. Please review!

#69.b) Write out a.k.a as "also known as"

Sure. Makes sense (this was moved to the main README doc.

#69.c) "single-site" and "multisite" seems wrong.

I agree that putting these two different phrasings side-by-side looks wrong, but "multisite" is the terminology used throughout the rest of Drupal core, as well as Drupal.org and even the group name on g.d.o; Above all, we need to be consistent. I don't see this being very trivial to change -- it should at least have its own issue. How about we just rephrase the sentence to say "single-site" in a different way, such that doesn't have a dash? Any ideas? I'm fresh out, and mentally exhausted.

jhodgdon’s picture

OK - let's just leave single-site and multisite as they are. Not a big deal. :)

Anyway... GREAT WORK! I think that all of these readmes are ready to go... with the one exception that we need to verify this part still:

+It is safe to organize modules into subdirectories, such as "contrib" for
+contributed modules, and "custom" for custom modules. Note that if you move a
+module to a subdirectory after it has been enabled, you may need to clear the
+Drupal cache so that it can be found.
+

I will ask in IRC for someone who knows about this to comment.

Also, of course anyone else who is following this issue is welcome to offer suggestions on these readme files, but I think they are really good. Thanks!

heyrocker’s picture

As far as I know, in Drupal 7 and below, the module's path is stored in {system} and not cleared on a cache clear. So if you move a module, you actually have to uninstall/reinstall the module or go into {system} and change the path by hand PLUS clear the cache.

In Drupal 8, once #1608842: Replace list of bootstrap modules, enabled modules, enabled themes, and default theme with config lands, this list will be built from the file system on system builds and cache clears so you won't have to do this anymore.

In IRC jhodgdon asked about the PSR-0 implications of this, and I don't really have an answer to that part but in theory it should just 'all work'. Worth trying though.

tim.plunkett’s picture

I moved views/ctools/devel from sites/all/modules to modules/ when I was testing #1724252: Allow and encourage top-level directories instead of /sites/all/* directories, and had absolutely no problems with the system table or PSR-0 classes.

heyrocker’s picture

Really? Well my information may be completely out of date then.

jwilson3’s picture

Another thought crossed my mind. I know its bad luck to second guess at the end, but anyway:

Should there be a statement in /sites/README.txt that describes the change from sites/all/modules and sites/all/themes to /modules and /themes respectively? or is that just going too far?

Eg.

It is now recommended to place your custom and downloaded extensions
(including modules and themes); in the /modules and /themes directories
respectively, located in the Drupal root.  The sites/all/ subdirectory
structure, which was recommended in previous versions of Drupal, will
be supported for compatibility and upgrade purposes.

TMI?

jhodgdon’s picture

RE #75 - I think this would be a good piece of information to put in the sites/README file. As written, it seems a slight bit redundant... perhaps something like this (if there isn't something already) -- a slight rewording:

It is now recommended to place your custom and downloaded extensions in the /modules, /themes, and /profiles directories located in the Drupal root. The sites/all/ subdirectory structure, which was recommended in previous versions of Drupal, is still supported.

RE #72-#74 - it sounds like for D8 recommending that you clear the cache is sufficient. Can someone do a quick test in D7 so we know what we'll need to do when we port the patch there?

jwilson3’s picture

StatusFileSize
new401 bytes
new6.63 KB
PASSED: [[SimpleTest]]: [MySQL] 41,329 pass(es).
[ View ]

Adding feedback from #76. Marking needs review just to get the automated review...

So the second part of #76 doesn't hold things up, could we postpone the investigation of verbiage for d7 backport after it gets to "patch to be ported" status?

jwilson3’s picture

Status:Needs work» Needs review

#facepalm

jhodgdon’s picture

Status:Needs review» Needs work

Looks good! There are a couple of minor things I still think we should clean up:

a) Terminology -- install profiles vs. installation profiles (I think it should always be installation)... The profiles README uses both terms. Also in the top-level README, I think we should use the term "installation profiles" consistently instead of switching between them.

b) I'd also like to see consistency in using / or not at the beginning of directories. For instance, this is from the sites README:

/**
It is now recommended to place your custom and downloaded extensions in the
/modules, /themes, and /profiles directories located in the Drupal root. The
sites/all/ subdirectory structure, which was recommended in previous versions
of Drupal, is still supported.

See core/INSTALL.txt for information about single-site installation or
multisite configuration.

I think maybe the / should be removed from /modules etc. since it's not on sites/all or core/INSTALL.txt?

c) In the top-level README, this first sentence doesn't seem to belong as part of the rest of this paragraph:

There are two basic install profiles provided with Drupal core by default.
Install profiles from the Drupal community are often bundled into
"distributions", which define groups of extensions (modules & themes) and
...

I think it fits better with the previous paragraph instead.

d) In the top-level README, say "contributed" (modules/themes) not "contrib".

e) These two paragraphs in the top-level README ... they seem to be saying the same thing and I think they could be combined?

Install profiles from the Drupal community are often bundled into
"distributions", which define groups of extensions (modules & themes) and
installation instructions that modify the installation process to provide a
site initialized for a specific use case, such as a CMS for media publishers, a
web-based project tracking tool, or a full-fledged CRM for non-profit
organizations raising money and accepting donations.

Note that some distributions may be downloaded as a single all-inclusive package
containing Drupal core, additional contrib and custom modules, themes, and an
installation profile; while others may simply provide a single install profile,
which should be downloaded and placed in the /profiles directory, leaving it up
to you to then also download the required modules and dependencies for proper
installation.

f) And yes I agree we can figure out D7 when we're ready to port this patch.

jwilson3’s picture

Status:Needs work» Needs review
StatusFileSize
new3.97 KB
new6.53 KB
PASSED: [[SimpleTest]]: [MySQL] 41,696 pass(es).
[ View ]

a) Terminology -- install profiles vs. installation profiles.

I agree it should be consistent, and agree we should go with installation profile (which is more natural sounding language). I decided to consult what the codebase uses so see if that would sway the vote, but sadly, there is no consensus or consistency there either:

* 34 instances of "install profile"
* 38 instances of "installation profile"

Most, if not all, of the references to "install profile" are in code comments, while "installation profile" is used in user-facing strings, but there are exceptions. core/includes/install.inc uses "install profile" consistently while core/includes/install.core.inc uses "installation profiles". CHANGELOG.txt uses "install profile". The two core installation profiles both use "install profile".

A Google search for "drupal install profile" turns up the top 6 results having "installation profile" in their title.

A search on the issue queue shows that more developers call them install profiles than installation profiles, presumably for brevity.

For now, I went with "installation profile", for readability. Someone feeling especially pedantic could open a separate issue to bring consistency here to the existing files. ;)

b) Consistently use (or not use) the leading slash at the beginning of directories.

Not fixed. I brought this up before myself. Prior to my patches, the top-level README is using leading slash for top level directories like /themes and /modules. Other references in the code to sites/all directories do NOT use the leading slash. The problem is that these two ways of referencing directories have never been put side-by-side, and thus never noticed until now. Removing the leading slash in the sites README would then make it inconsistent with the existing references to the same directories in top-level README. I'm still not sure what to do here. We need to decide whether code-wide consistency is more important than same-file consistency, and to either standardize on the leading slash in documentation for all of core, or just go with whatever works in a local context. It sounds like you're saying local context is more important, but I want to be sure about this.

c) First sentence in second paragraph doesn't belong.

Fixed. Moved it to the first paragraph.

d) ) In the top-level README, say "contributed" (modules/themes) not "contrib".

Fixed.

e) Combine installation profile paragraphs in top level README

Revised, though I'm still not happy that there is so much here. Maybe someone else needs to take a whack at trimming the three paragraphs down to something brief.
Also, I'm not 100% sure if what I've written is still valid. Its all a bit fuzzy to me. It looks like Drupal.org is in a process of doing away with naked install profiles and package everything up as distribution tarballs that include Drupal core and all dependencies. Is this correct?

jhodgdon’s picture

Status:Needs review» Needs work

- Sounds like we need to file a separate issue for install profiles vs. installation profiles terminology throughout core.
- Sounds like we need to file a separate issue about the consistency of using / or not in naming files and directories throughout core.
- I wasn't sure either about whether we distribute both bare installation profiles and distributions, but I checked project/distributions and I found at least one bare and one non-bare, so it looks like in practice we do both. I don't maintain a distribution so I am not sure how that is decided... anyway it looks like we need to talk about both possibilities.

Anyway, I think the text in the patch looks pretty good, with the file naming caveat ignored, and we're down to a couple of nitpicks:

a) Whenever you use which, it should be preceded by a comma, such as (core README):

+Installation profiles from the Drupal community are commonly bundled as
+"distributions" which modify the installation process to provide a website

b) I don't think it's right to call something a "distribution" if it doesn't include Drupal Core and all the stuff:

+Some distributions are packaged as all-inclusive zip files that include
+Drupal core, an installation profile, as well as any other required extensions,
+including contributed and custom modules, themes, and third-party libraries.
+Other distributions provide the installation profile alone, which should be
+downloaded and placed in the /profiles directory. The required extensions for
+proper installation should then be downloaded separately and placed inside the
+profile directory.

And it's also a bit redundant with the previous paragraph... So I think it would make more sense to combine this with the previous paragraph, and instead say something like:

Installation profiles from the Drupal community modify the installation process to provide a website for a specific use case, such as a CMS for media publishers, a web-based project tracking tool, or a full-fledged CRM for non-profit organizations raising money and accepting donations. They can be distributed as bare installation profiles or as "distributions". Distributions include Drupal core, the installation profile, and all other required extensions, such as contributed and custom modules, themes, and third-party libraries. Bare installation profiles require you to download Drupal Core and the required extensions separately; place the downloaded profile in the /profiles directory before you start the installation process.

c) Remove the period here (for consistency with other bullet points and to make sure it's not part of the URL):

+* Read about the difference between installation profiles and distributions:
+  http://drupal.org/node/1089736.

d)

+++ b/sites/README.txt
@@ -0,0 +1,11 @@
+
+This directory structure contains the settings and configuration files specific

I don't think we want a blank line at the beginning of the file (or any of the other files for that matter)?

jwilson3’s picture

Status:Needs work» Needs review
StatusFileSize
new3.81 KB
new6.43 KB
PASSED: [[SimpleTest]]: [MySQL] 41,897 pass(es).
[ View ]

This one includes all the changes in #81, with the exception of "81.a", which was no longer needed after the rewrite from "81.b".

jhodgdon’s picture

Version:8.x-dev» 7.x-dev
Status:Needs review» Patch (to be ported)

Thanks all! I committed the latest patch to 8.x. Let's see what we can do for 7.x -- obviously we don't have the top-level directories, but we could put the Installation Profiles section into the main README, and some of the language from the other readmes into the sites readme file(s).

jwilson3’s picture

StatusFileSize
new6.24 KB
PASSED: [[SimpleTest]]: [MySQL] 39,469 pass(es).
[ View ]

Great!

Here's a first attempt at D7 backport.

jhodgdon’s picture

Status:Patch (to be ported)» Needs work

This patch brings up a question for me. In D7, I frequently have to put things into sites/all/libraries (such as WYSIWYG editor scripts, etc.). Where do they go in D8? And does the README structure we were just working on say anything about that?

In this patch particularly, I'm not sure about this:

--- a/sites/all/README.txt
+++ b/sites/all/README.txt
@@ -1,7 +1,7 @@
+Place downloaded and custom modules, themes, and third party libraries in this
+directory to ensure clean separation from the Drupal core codebase and to
+facilitate safe, self-contained code updates.

That could be read to say that the modules etc. should go directly into sites/all rather than in sites/all/modules etc. Maybe a slight reword would be useful?

Other that that, I think it's probably OK... although it seems a bit redundant to have a lot of the same information in sites, sites/all, and sites/all/modules&themes... Is there some way we can avoid redundancy without making the information too hard to find?

jwilson3’s picture

This patch brings up a question for me. In D7, I frequently have to put things into sites/all/libraries (such as WYSIWYG editor scripts, etc.). Where do they go in D8?

The same question came to my mind too, actually. And it goes further.

My typical d7 sites/all folder structure looks like:

-sites
  |-all
    |-drush
    |-modules
    |  |-contrib
    |  |-custom
    |  |-features
    |-libraries
    |-themes
    |-translations

sites/all/libraries is not an officially required part of core, but I mentioned it in the D7 readme, which could arguably be removed. However, sites/all/translations *is* part of core, but not mentioned. Its not obvious if /translations directory has also been moved to drupal root. There is also now sites/all/drush in latest versions of drush.

I wonder what will happen to all of these, and if the new recommendations to move stuff out of sites/all into the Drupal root isnt addopted by drush and libraries, then we're opening a can of worms here. OMGFUD!

Maybe this was already discussed on the issues where the code change to move to /modules and /themes was implemented(?)

Agreed about redundancy between readmes in sites, sites/all, and sites/all/modules|themes, perhaps we dont need to add *all* the files from D8 after all, or maybe just move sites/all/README.txt to sites/README.txt and generalize everything there?

jhodgdon’s picture

Well. Let's stick with the D7 patch for now...

Regarding libraries, I guess they have to stay where they are. Core doesn't really have a concept of "libraries", it's just that some contrib modules look in sites/all/libraries for things like WYSIWYG editors, the Grammar Parser PHP code (using the contrib Libraries API module), and other stuff. They'll probably continue to look there, since there's no top-level libraries directory. So I think our READMEs are OK for D8 on that account.

Regarding translations... Actually, I think your sites/all/translations directory structure is incorrect (doesn't match the documentation I found)... but I think there are issues with how we instruct people to translate Drupal, so I've filed a separate issue:
#1802322: Translation instructions are incomplete

Anyway... I think we should leave the D8 readmes how they are for now on this issue, and see what we can do with D7.

jwilson3’s picture

StatusFileSize
new1.47 KB
new6.22 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch sitesreadme-1539940-89-D7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Incorporating feedback for D7 backport, from #86 thru #88.

I've removed sites/all/README.txt, and moved the contents into sites/README.txt. Hopefully, this clears up both the issues on #86 about duplicate info between these two files, and about poorly phrasing how modules and themes should be organized inside sites/all, by simply directing people to read the READMEs in those respective folders.

jwilson3’s picture

Status:Needs work» Needs review
jhodgdon’s picture

Assigned:Unassigned» jhodgdon
Status:Needs review» Reviewed & tested by the community

I think this structure makes a lot of sense, and the text is all good. Thanks! I'll get it committed shortly.

webchick’s picture

Category:feature» task

Re-categorizing as a task; no reason for this nice improvement to be subject to thresholds.

jhodgdon’s picture

Status:Reviewed & tested by the community» Fixed

Thanks all! Committed to 7.x.

David_Rothstein’s picture

Status:Fixed» Needs review
+It is safe to organize modules into subdirectories, such as "contrib" for
+contributed modules, and "custom" for custom modules. Note that if you move a
+module to a subdirectory after it has been enabled, you may need to clear the
+Drupal cache so that it can be found.

For a lot of Drupal 7 modules, clearing caches is enough, but for some it's not and your site will be hosed in such a way that even clearing caches doesn't fix it (see also Berdir's comment in #34). There's an issue somewhere that would make it so you can visit update.php to get out of the problem, but that was a complicated issue and the patch hasn't been committed yet. So I think Registry Rebuild is the only foolproof solution (and even that, I've seen rare cases where it fails).

So in short, moving enabled modules between subdirectories is a lot more fragile in Drupal 7 than it is in Drupal 8 and as a result I'm not sure the above text is quite correct?

David_Rothstein’s picture

That was a crosspost... but same questions apply either way.

jhodgdon’s picture

Status:Needs review» Needs work

OK, sounds like we need a quick update patch to fix the docs that were committed.

jwilson3’s picture

I'd be happy to rewrite it, but I'm not quite sure what to say exactly. Could we replace it with a link to a doc page on how to troubleshoot this?

Eg.

Be aware that moving a module to a subdirectory after it has been enabled is known to cause issues in various cases. See http://drupal.org/node/X for further information and troubleshooting help.

Is there such a doc page?

jhodgdon’s picture

We don't probably have a page... How about just changing the text so that instead of

Note that if you move a
+module to a subdirectory after it has been enabled, you may need to clear the
+Drupal cache so that it can be found.

it says something like "Note that moving a currently-enabled module to a different directory can cause problems, so if you want to move a module, disable it first and then re-enable it after the move." I mean, if they can get by with just clearing the cache, fine, but this will for sure be safe, right?

jhodgdon’s picture

Assigned:jhodgdon» Unassigned
dangswiss’s picture

Assigned:Unassigned» dangswiss

I'll take a stab at the wording.

LewisNyman’s picture

It's worth including this bug in the fix once you guys have agreed on the correct wording:
#1919590: README.txt references /sites/all instead of /modules and /themes

jhodgdon’s picture

We don't want that wording in the D7 version. D7 still uses sites/all, not the top-level modules/themes directories.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 89: sitesreadme-1539940-89-D7.patch, failed testing.

jhodgdon’s picture

This is actually being worked on right now in another issue for Drupal 8. See discussions there.

darol100’s picture

Status:Needs work» Needs review
StatusFileSize
new6.64 KB
PASSED: [[SimpleTest]]: [MySQL] 41,478 pass(es).
[ View ]

Here is a patch for D7 base on the D8 version.

darol100’s picture

Should we include the sites/all/libraries/README.txt into this issue ? D7 recently add the libraries folder into core/ #667058: Add an 'empty' libraries folder (or similar) and encourage people to use it properly

darol100’s picture

Assigned:dangswiss» Unassigned
jhodgdon’s picture

Status:Needs review» Postponed

I think we should postpone this issue until #2115737: Make the text in modules, themes, and profiles README.txt files more user-friendly is finished. They are overlapping. Either that or close one of these as a duplicate of the other.

darol100’s picture

Status:Postponed» Needs work
StatusFileSize
new6.38 KB
PASSED: [[SimpleTest]]: [MySQL] 41,480 pass(es).
[ View ]

Since #2115737: Make the text in modules, themes, and profiles README.txt files more user-friendly is been already commit to Drupal 8 Core, I started working on this issue. Just like, I mention on #107 Should we include the sites/all/libraries/README.txt into this issue ? D7 recently add the libraries folder into core/ #667058: Add an 'empty' libraries folder (or similar) and encourage people to use it properly .

I have created a backport patch from #2115737: Make the text in modules, themes, and profiles README.txt files more user-friendly.

darol100’s picture

Status:Needs work» Needs review

Sorry wrong Status.

jhodgdon’s picture

Hm.

So ... Right now it looks like in Core, I am not seeing that there is any sites/all folder there at all, at least if I do a git pull. Is this patch creating these folders by adding README files to them?

Also, I don't understand why this patch includes the (top level)/profiles/README.txt file that is already there.

And I don't see a libraries folder anywhere.

So I'm a bit confused?

jhodgdon’s picture

Oh duh. This is now a 7.x issue. ;)

So yes, I think we should also create a nice libraries README.

I'm not sure about the subdirectories there though, because I think that a lot of the 7.x modules that require libraries require them to go into specific directories under libraries.

darol100’s picture

Assigned:Unassigned» darol100
darol100’s picture

Status:Needs review» Needs work
darol100’s picture

+++ b/sites/all/modules/README.txt
@@ -1,16 +1,38 @@
+all sites. You may also put modules in the sites/all/modules directory, and the
+versions in sites/all/modules will take precedence over versions of the same

This sentence does not apply for Drupal 7 because this README.txt is already in sites/all/modules. So I have remove it.

+++ b/sites/all/themes/README.txt
@@ -1,14 +1,31 @@
+all sites. You may also put themes in the sites/all/themes directory, and the
+versions in sites/all/themes will take precedence over versions of the same

Same in here.

Here is a patch;however, this would need some work:

I do not know what we should mention in the sections "DOWNLOAD ADDITIONAL LIBRARIES" and "MORE INFORMATION". We do not have a place in Drupal.org to download libraries. In addition, we do not have section that talks about libraries in the /README.txt.

Should we mention about github/bitbucket in the "DOWNLOAD ADDITIONAL LIBRARIES"? Most of libraries (that I have used) are stored on Github.

Should we add a section in the /README.txt that talks about libraries ? Or we could reference an external link https://www.drupal.org/documentation/modules/libraries ? The problem with the external link is that it would not have a consistency with the rest of the README.txt

darol100’s picture

Assigned:darol100» Unassigned
jhodgdon’s picture

The README files in D8 were going into top-level modules, themes, and profiles directories. These do not exist in 7 (well they do, but they're for the Core files).

So the README files for 7 need to be adapted a bit, because they will be in sites/all/* folders. Sounds like you know this and have already done it, great!

Regarding libraries, ... right, there is not a central place to download them on drupal.org. They come from:
a) Some are outside downloads. For instance if you want to install the CKEditor to work in the WYSIWYG module in D7, you go to ckeditor.org or somewhere like that and download it, and put that into the libraries folder in a specific location. Or Github or Bitbucket or SourceForge or whatever.
b) Some are "module" projects on Drupal.org, which instead of installing in the normal sites/all/modules location, you are told to install in sites/all/libraries instead.

We could put something like that into the readme, but really the answer is "put things in Libraries if the instructions for a Drupal module tell you to do so, and follow their instructions for where exactly to put it regarding folder names etc.".

Maybe you can figure out how to word this? I dont' think we should mention Bitbucket etc. by name, but we should probably say something about external downloads and some Drupal module projects.

David_Rothstein’s picture

Status:Needs work» Fixed

Looks like good progress, but since this is a backport of #2115737: Make the text in modules, themes, and profiles README.txt files more user-friendly can we move it there so the issue credit works out properly? (I'll reopen that issue for Drupal 7 now.)

This issue was really only open due to a crosspost between #93 and #94 two and a half years ago :) If I hadn't cross-posted with the original commit, I might have just created a new issue for #94 instead. So I'll do that now too, and we can close this.

David_Rothstein’s picture

jhodgdon’s picture

Status:Fixed» Closed (duplicate)

In that case probably the right status here is 'duplicate' right?

David_Rothstein’s picture

Status:Closed (duplicate)» Fixed

Well, the original issue was fixed via a commit in #93, so I am voting for "fixed" :)

Status:Fixed» Closed (fixed)

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