Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comments
Comment #1
jhodgdonHmmm... 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.
Comment #2
chrisparsons CreditAttribution: chrisparsons commentedJust 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.
Comment #3
tim.plunkettI 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.
Comment #5
jhodgdonHere'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?
Comment #6
jhodgdonIn 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?
Comment #7
webchick#5 sounds good, and would be a novice task.
Comment #8
infiniteluke CreditAttribution: infiniteluke commentedHere's a stab at the wording. I kept it simple without any examples, as I felt adding examples would imply promotion of the examples.
Comment #9
mrf CreditAttribution: mrf commentedGuess 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.
Comment #10
hefox CreditAttribution: hefox commentedre drush: Drush will put downloaded contrib modules into contrib on drush dl if it exists to my memory.
Comment #11
BerdirPlacing 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.
Comment #12
jhodgdonThe 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?
Comment #13
infiniteluke CreditAttribution: infiniteluke commentedRe #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?
Comment #14
jhodgdonThere 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...
Comment #15
infiniteluke CreditAttribution: infiniteluke commentedIs 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.
Comment #16
jhodgdonLet's just do all the README's here on this one issue. And yes, I think we can add one to the sites directory.
Comment #17
infiniteluke CreditAttribution: infiniteluke commentedComment #18
infiniteluke CreditAttribution: infiniteluke commentedFixed sites/all/themes README to read "It is safe to organize themes into sub-directories."
Comment #19
jhodgdonRE #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).
Comment #20
infiniteluke CreditAttribution: infiniteluke commentedReworded as mentioned in #19.
Comment #21
infiniteluke CreditAttribution: infiniteluke commentedComment #22
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedJust a note: Why are we breaking the line before 80 characters? I see that in the context, too - not sure why.
Comment #23
jhodgdonGood point, it looks like that line can/should include sub-directories without going over 80 characters. The other lines should be checked for that.
Comment #24
infiniteluke CreditAttribution: infiniteluke commentedLine 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
Comment #25
ryan.gibson CreditAttribution: ryan.gibson commentedLines don't go over 80.
Comment #26
jhodgdonThere is a minor grammatical error in this patch:
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.
Comment #27
infiniteluke CreditAttribution: infiniteluke commentedI 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.
Comment #28
jwilson3I'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)
Comment #29
jwilson3Another 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.
Comment #30
jhodgdonRE #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.
Comment #31
rsgracey CreditAttribution: rsgracey commentedSo 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?
Comment #32
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedSure. 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.
Comment #33
ro-no-lo CreditAttribution: ro-no-lo commentedI 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.
Comment #34
BerdirJust 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.
Comment #35
jwilson3Per 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.
Comment #36
jhodgdonWow, 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:
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:
- 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.
e) Formatting in the profiles readme near the end:
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.)
Comment #37
jwilson3Excellent 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.
Comment #38
jwilson3This 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:
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).
Comment #39
jwilson3A 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.
Comment #40
jwilson3OMG I missed an "Alternatively"... #facepalm I think its ready now..
Comment #41
jwilson3In 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.
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".
Comment #42
ryan.gibson CreditAttribution: ryan.gibson commentedA few suggestions and fixes here.
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."
Is this meant to be "...pattern may be used to restrict modules..." instead of "restricted"?
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."
missing article
"...used to restrict a profile's availability..."
I don't think we need a dash in subdirectories.
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..."
Comment #43
jhodgdonGreat 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?)
Comment #44
jwilson3Incorporated 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.
Comment #45
jwilson3Column wrapping was off on one of the lines that was changed...
Comment #46
jwilson3Hrm, 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.Comment #48
jwilson3Grr. 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
.Comment #49
Niklas Fiekas CreditAttribution: Niklas Fiekas commented(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.)
Comment #50
jwilson3@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:
But this doesn't apply, it needs to use /dev/null
(/me shrugs)
Comment #51
Niklas Fiekas CreditAttribution: Niklas Fiekas commentedOh, 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. Thengit 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 strictgit apply
, that notices that the file does not exist in the target repo. Other tools likepatch -p1
aren't that strict.Cut. Back to the actual issue :)
Comment #52
ryan.gibson CreditAttribution: ryan.gibson commentedThis is starting to look good.
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.
Comment #53
jwilson3If 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?
Comment #54
jhodgdonOK, 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."
Comment #55
ryan.gibson CreditAttribution: ryan.gibson commentedWell, 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."
Comment #56
jwilson3What about "modify"?
Eg,
"This directory should be used to keep downloaded and custom themes that modify the appearance of your site beyond Drupal core."
Comment #57
jhodgdonchange/modify both sound fine to me.
Comment #58
ryan.gibson CreditAttribution: ryan.gibson commentedModify works for me!
Comment #59
jwilson3re 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.
Comment #60
jwilson3re: "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.
Comment #61
jwilson3Modify it is (for now anyway)!
Comment #62
jwilson3Given #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."
Comment #63
jwilson3Here'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).
Comment #64
jwilson3Here's a more complete thought with simple, straightforward sentences.
Thoughts? Should I roll with this?
Comment #65
jwilson3So... 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:
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.
Comment #66
jhodgdonThanks, 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:
"freely available for download and usage" -- ?!? How about just "available for download".
b) Did anyone actually check on the part about:
?
Comment #67
jwilson3What... 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)?
Comment #68
jwilson3Comment #69
jhodgdonI 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 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"...
Comment #70
jwilson3I 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!
Sure. Makes sense (this was moved to the main README doc.
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.
Comment #71
jhodgdonOK - 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:
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!
Comment #72
gddAs 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.
Comment #73
tim.plunkettI 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.
Comment #74
gddReally? Well my information may be completely out of date then.
Comment #75
jwilson3Another 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.
TMI?
Comment #76
jhodgdonRE #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?
Comment #77
jwilson3Adding 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?
Comment #78
jwilson3#facepalm
Comment #79
jhodgdonLooks 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:
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:
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?
f) And yes I agree we can figure out D7 when we're ready to port this patch.
Comment #80
jwilson3I 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. ;)
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.
Fixed. Moved it to the first paragraph.
Fixed.
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?
Comment #81
jhodgdon- 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):
b) I don't think it's right to call something a "distribution" if it doesn't include Drupal Core and all the stuff:
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):
d)
I don't think we want a blank line at the beginning of the file (or any of the other files for that matter)?
Comment #82
jwilson3This one includes all the changes in #81, with the exception of "81.a", which was no longer needed after the rewrite from "81.b".
Comment #83
jwilson3Follow-up issues from #81:
#1799116: [policy/patch] Standardize on "installation profile" instead of "install profile"
#1799126: [meta] Should a leading slash denote top-level folders (/themes /modules /sites/all /core etc)
Comment #84
jhodgdonThanks 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).
Comment #85
jwilson3Great!
Here's a first attempt at D7 backport.
Comment #86
jhodgdonThis 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:
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?
Comment #87
jwilson3The same question came to my mind too, actually. And it goes further.
My typical d7 sites/all folder structure looks like:
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?
Comment #88
jhodgdonWell. 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.
Comment #89
jwilson3Incorporating 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.
Comment #90
jwilson3Comment #91
jhodgdonI think this structure makes a lot of sense, and the text is all good. Thanks! I'll get it committed shortly.
Comment #92
webchickRe-categorizing as a task; no reason for this nice improvement to be subject to thresholds.
Comment #93
jhodgdonThanks all! Committed to 7.x.
Comment #94
David_Rothstein CreditAttribution: David_Rothstein commentedFor 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?
Comment #95
David_Rothstein CreditAttribution: David_Rothstein commentedThat was a crosspost... but same questions apply either way.
Comment #96
jhodgdonOK, sounds like we need a quick update patch to fix the docs that were committed.
Comment #97
jwilson3I'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.
Is there such a doc page?
Comment #98
jhodgdonWe don't probably have a page... How about just changing the text so that instead of
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?
Comment #99
jhodgdonComment #100
gurtner CreditAttribution: gurtner commentedI'll take a stab at the wording.
Comment #101
LewisNymanIt'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
Comment #102
jhodgdonWe don't want that wording in the D7 version. D7 still uses sites/all, not the top-level modules/themes directories.
Comment #105
jhodgdonThis is actually being worked on right now in another issue for Drupal 8. See discussions there.
Comment #106
darol100 CreditAttribution: darol100 as a volunteer and commentedHere is a patch for D7 base on the D8 version.
Comment #107
darol100 CreditAttribution: darol100 as a volunteer and commentedShould we include the sites/all/libraries/README.txt into this issue ? D7 recently add the libraries folder into core/ #667058: Add a libraries folder with a README.txt in it to DRUPAL_ROOT
Comment #108
darol100 CreditAttribution: darol100 as a volunteer and commentedComment #109
jhodgdonI 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.
Comment #110
darol100 CreditAttribution: darol100 as a volunteer and commentedSince #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 a libraries folder with a README.txt in it to DRUPAL_ROOT .
I have created a backport patch from #2115737: Make the text in modules, themes, and profiles README.txt files more user-friendly.
Comment #111
darol100 CreditAttribution: darol100 as a volunteer and commentedSorry wrong Status.
Comment #112
jhodgdonHm.
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?
Comment #113
jhodgdonOh 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.
Comment #114
darol100 CreditAttribution: darol100 as a volunteer and commentedComment #115
darol100 CreditAttribution: darol100 as a volunteer and commentedComment #116
darol100 CreditAttribution: darol100 as a volunteer and commentedThis sentence does not apply for Drupal 7 because this README.txt is already in sites/all/modules. So I have remove it.
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
Comment #117
darol100 CreditAttribution: darol100 as a volunteer and commentedComment #118
jhodgdonThe 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.
Comment #119
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedLooks 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.
Comment #120
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedCreated the issue.
Comment #121
jhodgdonIn that case probably the right status here is 'duplicate' right?
Comment #122
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedWell, the original issue was fixed via a commit in #93, so I am voting for "fixed" :)