backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

Update the update instructions #529

Closed jenlampton closed 9 years ago

jenlampton commented 9 years ago

As a follow-up to https://github.com/backdrop/backdrop-issues/issues/514 we should revisit and update the instructions that appear on /core/update.php?op=info. These have always been misleading (backup your code, wut?!) and need a few updates for CMI anyway.

These should closely match the instructions on https://backdropcms.org/guide/upgrade

ghost commented 9 years ago

Here's an initial attempt at this. Looks like:

update-php

quicksketch commented 9 years ago

Thanks @BWPanda! The lack of a description on Database makes it seem like Database should be self-explanatory, but I'm not sure it is to all administrators.

Perhaps we could read out the actual database name and config file location an provide that information directly here.

Grammatically, we should use "back up" when using as a verb, and "backup" when using as a noun.

How would you feel about nixing the entire section about modules? Or is that too presumptuous to expect users to know which version they were upgrading from?

ghost commented 9 years ago

That's a good idea @quicksketch, I've added a new commit to the PR that displays the DB name and config path. I also got rid of the module instructions as you're right: module files are available online so they don't need backing up by users.

Here's what the Update page now looks like: update

jenlampton commented 9 years ago

Wow, this looks great :) nice job on these @BWPanda

quicksketch commented 9 years ago

This looks great! All the code looks great too.

The only suggestion I have is that we shouldn't try to trim the "/active" directory off the end of the active config directory. By default, we keep config in files/config_[hash]/active, but a user could just as easily set a path like ../active_config as their config location, in which case this message would tell the user to back up ../, which doesn't make any sense.

Besides that, the staging config directory will usually be empty and not require backing up anyway, so let's just leave the active directory path as-is in the message.

Great work on the text, a definite improvement throughout.

ghost commented 9 years ago

...we shouldn't try to trim the "/active" directory off the end...

I didn't want to presume this, but now that you've suggested it, it does make it simpler :) Rebased and merged commits together in PR.

quicksketch commented 9 years ago

I double-checked the behavior of this when upgrading from Drupal 7 and it wasn't tremendously awkward. The active config directory is empty at the time of an upgrade, but at least the value is populated and the sentence makes sense. We should consider a separate update text entirely when upgrading from Drupal 7, but this as-is looks great. Merged in. Thanks again @BWPanda!