As mentioned in #1548034: Show permissions defined by a module after pm-enable, and in pm-info, it would be desirable to add drush_role into core. I looked at this a while ago, and wondered about the use of classes in drush_role vis-a-vis our future idea to implement all drush commands as classes. Since the later is perhaps a ways off, I was wondering if we could bring drush_role in now, more or less as-is? I could role a patch with minor improvements if this is desired.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | drush_role-4.patch | 8.74 KB | greg.1.anderson |
| #3 | drush_role.patch | 8.75 KB | greg.1.anderson |
Comments
Comment #1
greg.1.anderson commentedc.f #1649600: Adjust permissions after sql-sync
Comment #2
psynaptic commentedThis would be great. Please add drush_role to drush core, if you have time!
Comment #3
greg.1.anderson commentedThe example committed in #1649600: Adjust permissions after sql-sync does not work in Drupal 7. Here is a patch that brings Drush Role into core and uses it to add D7 support to the sync_enable example. If this code looks acceptable for Drush core, I'll improve the docs and add a test.
Comment #4
greg.1.anderson commentedFixes for D6 postgres sites.
Comment #5
moshe weitzman commentedThis class hierarchy model is quite different from our current version engine model. Do we prefer this one? Is this really our way forward?
The command definition texts are lame as hell.
Comment #6
greg.1.anderson commentedRegarding #5, I had the same thought initially. I could re-implement it using the existing Drush version engine model, but don't know if that is worthwhile if we might switch to something else (e.g. symfony-based) soon.
I'll definitely fix up the docs once it's decided which direction to take the code.
Comment #7
jonhattanA generalization of this approach will be of great benefit for 'drupal' engine (commands/core/drupal/environment*.inc has a lot of duplication) and other commands highly based on drupal version (search, pm-info, user-*)
Comment #8
greg.1.anderson commentedI was looking at converting this to the current engine model today. For engines where the specific instance can be selected by cli options, the Drush engine system is good and valuable. For engines where there is only one kind of engine, no options, and the only purpose of the engine is to provide code variants for different versions of Drupal, I don't see any value in declaring the php file that contains the class in the 'engines' record.
My recommendation would be to leave Drush_Role as it is; since there are no options, I like the current implementation better than the engines implementations. I think that it is okay for the D6 and D7 code to be in the same file, but if we move to a Symfony classloader in Drupal at some point in the future, we could migrate to use that at that time. I would also hold off on re-implementing existing optionless engine code until we decide what our final classloader strategy is going to be.
Comment #9
moshe weitzman commentedOK, makes sense to me. Lets fix up the command definition and commit.
Comment #10
greg.1.anderson commentedCommitted with tests, help text, --simulated support and some refactoring. Please re-open if any changes are needed.
Comment #12
greg.1.anderson commentedDo we want these commands backported to Drush-5.x?
Comment #13
greg.1.anderson commentedI am now more interested in getting Drush 8.x-6.x released than porting this to 7.x-5.x; closing again.