In UPGRADE.txt, the explanation of the minor version updates process says that an update sometimes includes changes to settings.php. As far as I know, Drupal does not include settings.php (only default.settings.php), and will not update this file, because it has been created by the user, not the Drupal installation.

The attached patch removes this explanation.

Correct me if I'm wrong.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Tor Arne Thune’s picture

Status: Active » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

Good point, but this is not the right patch.

It should say that a minor version update might include changes to default.settings.php, and you should replace your settings.php with the new default.settings.php and copy in the changes.

Tor Arne Thune’s picture

Status: Needs work » Needs review
FileSize
1.41 KB

Ah yes, that would work better, thank you. Attached patch tries to explain this without many changes to the original text.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

That looks good to me! Thanks!

webchick’s picture

Status: Reviewed & tested by the community » Needs work
If that's the case, replace your old
+   settings.php with the new default one, and copy the site-specific entries
+   (especially the lines giving the database name, user, and password) from the
+   old settings.php to the new settings.php created from default.settings.php."

I got a bit turned around in this sentence. :) Is there a way to word this in a less confusing way? For example, giving people specific steps to follow in this instance?

jhodgdon’s picture

How about something like this:

If that's the case, follow these steps:
1) Make a backup copy of your settings.php file, with a different file name.
2) Make a copy of the new default.settings.php file, and name the copy settings.php (overwriting your previous settings.php file).
3) Copy the custom and site-specific entries from the backup you made into the new settings.php file. You will definitely need the lines giving the database information, and you will also want to copy in any other customizations you have added.

(This should be formatted appropriately for the rest of the file -- probably 1. rather than 1), or maybe bullet points with - ?)

Tor Arne Thune’s picture

Status: Needs work » Needs review
FileSize
1.66 KB

The suggested change by jhodgdon sounds good to me. It definitely makes it less confusing and more easy on the eyes/brain.

I added the points as - bullet points, because I think it makes the text easier to read, as more numbers can make it more heavy at first glance. It's also done further down in the document. I also added a line of spacing between the bullet points, because I see this was down elsewhere in the document. I'm not sure if this is overkill or not.

Tor Arne Thune’s picture

Title: UPGRADE.txt describes unnecessary and erroneous step » UPGRADE.txt describes erroneous step on settings.php upgrade
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Works for me, thanks!

Dries’s picture

Can't we:

1. Always recommend people to make a backup of their settings file?

2. Recommend people to read the release announcement as it might have special instructions?

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs work

RE #10 - I'm maybe not sure what you're asking.
- Drupal downloads do not overwrite the settings.php file
- I think we already recommend that people make a backup of their db and files in general before they start
- I think we need these specific instructions on how to merge in a new default.settings.php - and the previous instructions were confusing.

So what I think I'm getting from #10 is that we should somewhere recommend that people always check the release announcement to see if there are any special instructions for upgrading to this version. And I think maybe we should clarify when we say that what the "release announcement" is and where to find it (i.e. if you are on the Drupal project page the link is I think "Notes").

Tor Arne Thune’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
1.65 KB

This patch has also been updated with the newest HEAD, after #1029346: INSTALL.txt and UPGRADE.txt should mention that you can download Drupal as a .zip file also was committed.

I'm not sure I follow your comment in #10.

jhodgdon’s picture

see #11 then. I didn't quite get #10 either. :)

Tor Arne Thune’s picture

Title: UPGRADE.txt describes erroneous step on settings.php upgrade » UPGRADE.txt describes erroneous step and should mention release announcement
FileSize
2.71 KB

Okay, I think I follow now. People should definitely be encouraged to read the release announcement before upgrading. It does seem strange that it's not mentioned in the document (in a general upgrade sense), so the attached patch tries to add some heads-up for the release announcement. I'm not sure it's worded correctly, so please, chime in :)

Tor Arne Thune’s picture

Status: Reviewed & tested by the community » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

Since the link on the project page is called "Notes", maybe we should refer to it as "the release notes" rather than "the release announcement"? If we call it "announcement", it could be confused with a d.o front page post announcing the release, which is not as important for them to read I think, and is not the node we are pointing out the link to.

Suggestion for the first added paragraph:

Each new release of Drupal has release notes, which explain the changes made since the previous version and any special instructions needed to update or upgrade to the new version. You can find a link to the release notes in the Downloads section of the Drupal project page (http://drupal.org/project/drupal), in the Links column for the version you are upgrading or updating to.

Then down below I would refer to this as release notes as well.

Tor Arne Thune’s picture

Title: UPGRADE.txt describes erroneous step and should mention release announcement » UPGRADE.txt describes erroneous step and should mention release notes
Status: Needs work » Needs review
FileSize
2.77 KB

I was hoping you would change the wording :) It sounds much better now, and I agree about the change from announcement to notes. I did a search for release announcement, and no other files use these words, so I think we are good, as it will be consistent in Drupal core.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

This looks good to me.

Tor Arne Thune’s picture

It would be great to get this in before the 8.x branch is created, as one step is plain-out wrong.

jhodgdon’s picture

jhodgdon’s picture

Version: 7.x-dev » 8.x-dev

Needs to go to d8 and then d7

Tor Arne Thune’s picture

Created D7 and D8 versions of the patch in #17 using Git. Just to try out patch creation in Git and hopefully get this committed.
The patches are identical and make the same changes as the one in #17.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Hm. I'm actually hesitant to put quite that much detail in how to get to the release notes. It's fully possible we want to revamp that UI at some point down the road, and we're almost certain to forget to change it here. Is it possible to vague that up a little bit?

Tor Arne Thune’s picture

I see your point and agree. In the new patches I have tried to rephrase this to be a bit less specific to the location of the link, but I feel it would be too much to remove the mention of where to find the release notes on Drupal.org entirely, as well as remove the link. I know personally that they can be a little difficult to find when starting out, as the location varies between communities. The link to the release notes on the Drupal project page reads "Notes," which could be missed as being the "release notes," but to further specify that the link reading "Notes" is the link to the "release notes" is a bit overkill. "Seek and you shall find" ;)

Tor Arne Thune’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1040810-UPGRADE-minor-version-updates-step-D7-2.patch, failed testing.

Tor Arne Thune’s picture

Status: Needs work » Needs review

Ah, I get it. The D7 patch will fail no matter what because the testbot tries to apply it to the 8.x branch. I should not have called it *D7-2.patch, but rather *D7.patch. Sorry for wasting resources!

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

The patches in #24 look good to me. Good idea webchick to make the instructions a bit less specific.

Tor Arne Thune’s picture

Great, all concerns and suggestions are in the patch and ready to go :)

xjm’s picture

Tagging issues not yet using summary template.

jhodgdon’s picture

jhodgdon’s picture

This has been sitting around for a while, so re-testing.

catch’s picture

Version: 8.x-dev » 7.x-dev

The less specific text looks good to me too. Committed to 8.x, back to 7.x for webchick to consider.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Looks good. Sorry, no idea how that fell off my radar for so long!

Committed and pushed to 7.x. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Needs issue summary update, -Needs backport to D7

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