Module name: Restore
Drupal core version: 7.x

Restore is a very simple set of modules:

* The main core module that will display the restore scripts and the current state of each.
* The differences module which provide a display of differences between the current state and the restore state.
* The generator module which provides an interface for exporting and generating restore scripts, either to the database, or script file.

http://drupal.org/sandbox/Orgun109uk/1809734
git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/Orgun109uk/1809734.git restore

Comments

bripatand’s picture

bripatand commented

Impressive module. Seems well written and designed.

There are some drupal coding standard issues in your module code.
For automated testing you can use http://ventral.org/pareview

I installed the module and I may be dumb but I have a hard time understanding how to use it.
It is a sophisticated module and I think it would deserve more explanation for the dummies like me...
For instance, I generated a module restore script but I had to look at the code to figure out what the operation would do.
I would suggest you describe some typical and basic use cases and how to implement them with your module.
Nothing fancy, but just the few steps to reach a specific backup/restore goal.

I also got a White Screen Of Death. I logged an issue for it.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

You need to set the status to "needs review" if you want to get a review.

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

NicA109’s picture

NicA109 commented
Status: Active » Needs review

Impressive module. Seems well written and designed.

Thanks :) been working on and off for a little while.

There are some drupal coding standard issues in your module code.
For automated testing you can use http://ventral.org/pareview

Thanks, i will have a look through it, and start working on some documentation.

I also got a White Screen Of Death. I logged an issue for it.

Thanks, should be fixed in the latest commit.

And thanks klausi, and i will try and have a look at some when i get back from work.

NicA109’s picture

NicA109 commented

Phew finally got the million and one warnings fixed from the pareview.

edit: http://ventral.org/pareview/httpgitdrupalorgsandboxorgun109uk1809734git

NicA109’s picture

NicA109 commented

Just an update since the initial post,

I have added a few more minor features (including db storage of scripts) and fixed some bugs i had found, added a few more operation classes and example restore scripts, another example module and added more documentation both in the UI and in the files.

paravibe’s picture

paravibe commented

Hello,

Some manual review:
restore.module: line 109, this line does not make sense, just remove this line and leave this
$output = '<p>' . t('This provides a list of available restore scripts and displays the status of the script.') . '</p>';
the same at line 118.
line 474 - 501, you often use this variable $status[$script_id]['operations'][$operation_class]['actions'][$action_id]['status'], maybe it make sense to announce a new short name variable and use it.

Everything else is good as for me. Nice work :)

NicA109’s picture

NicA109 commented

Hi,

Thanks for the review :)

I have made amendments, as you recommended, and done another automated review:
http://ventral.org/pareview/httpgitdrupalorgsandboxorgun109uk1809734git

Thanks :)

NicA109’s picture

NicA109 commented
Priority: Normal » Major
hhhc’s picture

hhhc commented

Hello,

some feedback.

* I would suggest to extend the documentation: the project page as well as README could be extended by a 2 sentence big picture explanation of what this module actually does. From my perspective, a more detailed explanation would be necessary if you would like a broader audience to use this module. I think I get the general idea of it but it really would require more information to really use it.
* admin/config/development/restore/diff shows differences between "authenticated user" and "administrator" although they were not changed. Is it a bug?
* The source code is very well documented!

Looks promising! Please continue.
Thanks

NicA109’s picture

NicA109 commented

Thanks for the feedback,

I have made some amends to the page and readme text to better explain the module and submodules.

I installed the modules on a fresh install and not getting the same problem, im guessing its probably to do with encoding, i have made the difference checking to not be so strict so hopfully helps.

Thanks :)

hhhc’s picture

hhhc commented

HI,

  • pls fix the coding issues as per http://ventral.org/pareview/httpgitdrupalorgsandboxorgun109uk1809734
  • trying to generate a new script. When I select module, and try to use the filter of modules it throws an Ajax error:
    Path: /system/ajax
    ReadyState: 4
    After clicking ok it forwards me to the code page showing the restore code.
  • I think that you might want to add at least 1-2 use cases explaining how to use this module. Even after having played with it for 15min now it is not 100% clear why I should use this one over features module, where the added value is and how to use it properly.

Please, don't misunderstand, I like the general idea but it is very complex for a new user who did not work with it to understand...
hhhc

NicA109’s picture

NicA109 commented

Thanks again for the review,

I have fixed all the issues from PAReview, except for one which seems to be a problem with code sniffer (?) as its telling me to use getdir which does not exist, and i can not find anything about such a function in the PHP docs.

939 | ERROR | dir() is a function name alias, use getdir() instead

I am unable to reproduce the problem you where having, it maybe because of some of the changes i had made. Have you tried to remove the module (disable and uninstall) and tried again?

I have updated the module page, and added a "help" page within the module to better explain the module and usage of it.

pirog’s picture

pirog commented

The code looks pretty good to me. Good formatting, standards compliance and documentation.

For the dir() error above, could you use file_scan_directory instead?

Some other minor issues i saw:

  1. You don't need to keep the .gitignore file
  2. The project page has a lot of really good stuff on it but as a consequence it is very long. You might want to consider breaking up the "documentation" aspects of your project into subpages or a separate "documentation" page.
  3. Might want to consider breaking the README up similarly to 2.
NicA109’s picture

NicA109 commented

Thanks :)

I have amended the code to use the file_scan_directory instead, and while i was there added some static caching around it.

Also added a documentation area on drupal.org (linked in the projects page) and broken up the README into INSTALL and TODO, and removed the git ignore file.

Many thanks,

sreynen’s picture

sreynen commented
Title: Restore » [D7] Restore
NicA109’s picture

NicA109 commented
Priority: Major » Critical
seworthi’s picture

seworthi commented
Assigned: Unassigned » seworthi
seworthi’s picture

seworthi commented
Assigned: seworthi » Unassigned

Review of code and checklist on http://ventral.org show's module is RTBC. Did not install and test.

seworthi’s picture

seworthi commented
Status: Needs review » Reviewed & tested by the community
kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented

This is quite a large module! Looks RTBC from me, but I will have to ask for a second opinion before fixing this issue. Overall nice quality and abstraction!

----
Top Shelf Modules - Enterprise modules from the community for the community.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
commented
Priority: Critical » Normal
Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, Orgun109uk!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

----
Top Shelf Modules - Enterprise modules from the community for the community.

NicA109’s picture

NicA109 commented

Great, many thanks to everyone :)

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Added git clone info.