[meta] Drupal Module Upgrader Battleplan-o-rama!

Linking some stuff.

Here's an update from our scrum call earlier today!

Adam's first task has been working on refactoring the module to make it both easier to contribute to and easier to maintain. He is keeping compatibility with PHPCS so we can remain in-sync with Coder Review and benefit from IDE integration, but are using new PHP OO hotness (traits, etc.) to break apart monolithic horror classes into nice bite-sized chunks. We figure that some upfront investment here will get us a lot better velocity going forward.

For example, instead of just a single InfoToYamlSniff, containing many turns and twisty passages, each logic check is broken into its own class like CoreVersionSniff and ObsoleteDependenciesSniff. Where there's common functionality that many sniffs might need, these have been pulled into traits like a ReaderTrait, WriterTrait, OncePerFileTrait, etc. This avoids a lot of copy/pasta code.

There's also now Drush integration which wraps phpcs / phpcbf with nice commands like drush dmu-analyze [modulename] and drush dmu-upgrade [modulename].

This work is all happening in the refactor branch. Testing and reviews welcome!

Between now and next week, Acquia has another Hackathon happening. Adam's going to join up with the team porting Acquia Connector module in order to get some "in the field" knowledge of what's needed by DMU, and Tim Plunkett is going to help us identify a suitable module from the Demo Framework distribution to use as a target module for DMU going forward. While Pants is cool and all, it doesn't really have much real-world value. ;)

That outline actually sounds really great to me. Very impressed with how fast this is coming together so far. :D

Report from my sync-up with Adam again today!

All the stuff from the refactor branch has now been merged into the master branch. It includes:

• .info.yml generation, .routing.yml generation, event subscribers for hook_init/exit, and controller class generation.
• Still leverages phpcs as the problem detection routines, to benefit from IDE integration/Coder Review UI, and to allow people to fix the problems manually if they'd like.
• However, no longer using phpcbf to convert the code, but rather a Drush plugin. Run drush dmu-upgrade target_modulename from the Drupal 8 root (after enabling the module and running composer install). No output == it worked ok. :)
• For conversions, we're using the pharborist library, which is both easier to read/write compared PHP Parser and especially raw PHP tokens, and also has an extremely active/enthusiastic maintainer (grom358). The library also handles whitespace, which will probably come in handy.
• Conversions themselves are implemented as Drupal 8 plugins. This will hopefully make the project much easier for Drupalists to contribute to, since this is a common pattern used in Drupal 8. You can see some examples under the src/Plugin/DMU folder. Here's HookMenu.php so you can get an idea of the syntax:
  

  
  namespace Drupal\drupalmoduleupgrader\Plugin\DMU;
  
  use Drupal\drupalmoduleupgrader\Converter\HookConverterBase;
  
  /**
   * @Converter(
   *  id = "HookMenu",
   *  description = @Translation("Converts Drupal 7's hook_menu() to shiny new routes."),
   *  hook = "menu"
   * )
   */
  class HookMenu extends HookConverterBase {
  
    /**
     * {@inheritdoc}
     */
    public function execute() {
      $hook = $this->module->getHook('menu');
  
      if ($this->containsLogic($hook)) {
        throw new \LogicException('Cannot convert a hook that contains logic.');
      }
  
      // Get the plugin manager for route converters.
      $manager = \Drupal::service('plugin.manager.drupalmoduleupgrader.route_converter');
  
      // Execute hook_menu() to get the router items.
      $router_items = $this->module->execute($hook->getName()->__toString());
  
      foreach ($router_items as $path => $router_item) {
        foreach ($manager->getDefinitions() as $id => $definition) {
          $plugin = $manager->createInstance($id)->setModule($this->module);
  
          if ($plugin->isExecutable($router_item)) {
            $plugin->execute($path, $router_item);
          }
        }
      }
    }
  
  }
  

• For classes and other things that will be written out multiple times, these are encapsulated in Twig in the templates folder. For example, Form.twig:
  

  
  namespace Drupal\{{ module }}\Form;
  
  use Drupal\Core\Form\FormBase;
  use Drupal\Core\Form\FormStateInterface;
  
  class {{ class }} extends FormBase {
  }
  

Dynamic elements are replaced during code write-out. Once again, this basic syntax should be familiar to Drupal 8 developers who want to contribute to the module and add other conversion routines.

---

Adam's now working on writing up some docs about the overall architecture which Wim asked for, then will try and get form generation working. Exciting!!!

Here's today's update!

- Docs, docs, docs! Both the project page and README was updated for the latest version of the code to help people get the sucker up and running, and Adam's made a great start at developer documentation explaining how everything works. YEAH!

- Lots of more work on various conversions. Here's a sample diff generated by the script as of 5 minutes ago. Covers .info.yml, .routing.yml, basics of page/form controller generation, block plugins, etc. HUGE thanks to grom358 for all of his help with Pharborist, a highly usable PHP parser, which is really coming along!

- Next up is working on function replacement stuff which will allow us to hit variable_*, global $user, $user->uid, theme(), etc.

- If various of those function replacement changes are made by hand, you actually start to see stuff happening now! Very exciting. :) Here's a block from a D7 module that's been converted with DMU showing up on a D8 site:

"End of sprint 1" update!

Just officially closed out #2320075: [meta] Get DMU working on Pants module again. Obviously, the module is not "working" working yet, but quite far enough, and even further than this project did in February! :)

Here's the current diff generated: https://www.drupal.org/files/issues/current-conversion_7.patch

As you can see from the diff, the following things are covered:

- .info => .info.yml
- variable_* -> config API
- Route conversions, including access callbacks
- Settings form conversions
- Blocks -> plugins
- A variety of simple find/replace rules

If you turn Pants module on, you get no errors; you can place and view the change pants block; the configuration form is functioning. YEAH!

Tagged a 0.1.0 release to refer back to, in order to free master up for moar refactoring!

Next up will be menu links, a smarter way to do entity property -> method conversions, and more! We'll also now start to test this against "real" module targets, the first one being Diff, on tim.plunkett's recommendation.

We demoed the project internally to some OCTO folk today to wrap things up for this sprint, and got some great feedback which we've captured in the following issues:

• #2325587: Ditch PHPCodeSniffer, in favor of "dmu-analyze" report containing similar thing
• #2325583: Rename "Module" plugin namespace to something more descriptive
• #2325597: Add relevant change notices to @fixme comments
• #2325607: [meta] Get DMU working on Diff module

The biggest thing there is obviously ditching PHPCodeSniffer entirely in favor of a "dmu-analyze" Drush command. Our main concern here was one of usability/contribute-ability. PHPCS makes the installation instructions for this project much more onerous than they need to be, and requires contributors to the project to duplicate their logic checks in two places (one for the "finding" and one for the "fixing") using two incompatible systems (one being investigating individual tokens which is roughly like fighting with both hands tied behind your back and both legs chopped off). Additionally, of our target audience, many more will have Drush installed than PHPCS, so it lowers the barrier to using it as well.

We still do definitely want to keep the reporting aspect, however. There's tremendous value for people new to Drupal 8 to understand the nature of what code changes have happened that affect their modules, and how the new systems work. So since phenaproxima made so much progress on the project this sprint outside of what we originally "scoped," it's likely that we'll shift some energy into this over the next two weeks.

More on Monday!

Updating the battle plan!

So last sprint phenaproxima actually got WAY further than we originally suspected, so re-jiggering the plan for this sprint.

This sprint, the focus shifts from the silly demo module Pants to a project of actual community significance, Diff. While that module has a D8 port already in progress, Diff exercises enough of the main Drupal APIs that we should be able to find lots of juicy things to work on there. ;)

We took a spin through the module code. Last sprint, phenaproxima got a good chunk of hook_menu() and hook_block_*() working. This sprint, we're diving deeper into those APIs, trying to handle e.g. MENU_LOCAL_TASK and other variants, page callbacks in include flies, and configurable blocks.

And finally, the major architecture overhaul to remove PHPCS and replace it with the dmu-analyze is big another area of focus. This code already exists in master.

Should be fun! :)

Penciling in some thoughts for next sprint as well, though note that this is subject to change.

Oops, haven't updated this in awhile. :)

#2325587: Ditch PHPCodeSniffer, in favor of "dmu-analyze" report containing similar thing is complete, although the output is pretty stark and needs some feedback on how to improve. Fortunately, we have an issue for that very thing here: #2328647: dmu-analyze's output is hideous

#2193649: Upgrade hook_menu() is coming along. It's generating menu links, local tasks, and actions, although there are still a few snags that Adam is working through atm, chiefly #2328827: Local tasks not appearing on admin/config/content/diff and #2331863: "Some mandatory parameters are missing errors" with formerly optional route parameters.

As an added bonus, it's also handling moving classes to PSR-4, and a few new function replacements. :)

Latest diff of conversions against Diff module is here: https://www.drupal.org/files/issues/current-conversion_11.patch

YESSSS! Check this out!

DMU now converting:
- Routes
- Menu links
- Local tasks
- Action links
- Probably other things I forget now.

Additionally, lots of work done upstream on Pharborist to allow things like commenting out entire chunks of code. This will be super useful later on.

Things that are left to do before demo tomorrow: https://www.drupal.org/project/issues/search/drupalmoduleupgrader?projec...

- Fix dmu-analyze #2335915: dmu-analyze: Illegal string offset 'page callback' HookMenu.php:176
- Get Parametric rewriters working again. #2336299: Get parametric rewriters working again
- If possible, get Diff's "Entities" tab to show up (e.g. by commenting out code)
- #2331849: Convert drupal_set_title()
- #2336291: Convert module_invoke_all()
- Figure out what got committed to 8.x core between last night and this morning broke the plugin registry #2336289: Get DMU working again against the latest D8

Home stretch!! :D

Sketching out some more battle plan ideas for the future, based on today's demo call. Will update this better on Monday.

Oh btw, in case you missed it on the Twitters. ;P Here is a ~5 min video of the most recent tagged release. https://www.youtube.com/watch?v=RbKjHbpdGYo Also adding to the project page momentarily.

Hello lovely people!

For this sprint, we chose to focus on prepping the module for Amsterdam, where it will be demoed at both a dedicated BoF session (https://amsterdam2014.drupal.org/bof/drupal-module-upgrader-help-collabo...) and incorporated briefly into the module upgrade lab at https://amsterdam2014.drupal.org/session/upgrade-your-module-drupal-8. That entails a couple of things:

- #2327335: [meta] Port automated tests to D8. This is very useful because if a module (such as Pants) took the time to write tests in advance of the port, by doing a few simple find/replace routines we're able to give module authors a complete list of what's left to fix in their module before tagging an 8.x release. Currently working for WebTestCase, UnitTestCase still a todo. Here's a screenshot of it mostly working:

- #2328647: dmu-analyze's output is hideous. One of the key benefits of this module to people who are new to Drupal 8 is a curated intro to the list of change records specifically for your module. The command-line way was getting too onerous, and so we went with prettified HTML output, which currently looks something like this:

Feedback on the exact format warmly welcome!

- #2340435: Create docs for DMU contributors Since one of the goals of Amsterdam is to get some new contributors involved with writing routines, we want to make sure this project is as easy to contribute to as possible. If you have any specific items you feel need to be addressed or questions that need answers, please post 'em there!

Adam has also started digging in on config schema and Twig, though those were both slotted for later sprints. Such an over-achiever. :D

Greetings, faithful followers! :)

DMU passed the formidable QA hurdles for this sprint and is now ready for download as 8.x-0.4!

Now that we're using standard contrib numbering schemes, you can just drush dl drupalmoduleupgrader and it'll grab it. :D (Don't forget to run composer install afterwards though!)

Here are the release notes:
- Nice, HTML-ified "analyze" report with linkified links, files/line numbers, etc.
- Covers conversion of automated (web) tests, to assist in porting modules with test coverage. Unit tests are not converted yet.
- Moves/splits classes to proper PSR-4 namespace.
- Generates FIXME messages for some things it can't do yet, and comments out some invalid function calls that cannot be converted (again, that depends on plugin coverage).
- A first crack at some of the common Entity API functions: entity_load/multiple, entity_save/delete, and entity type-specific variants: node_load, user_delete, etc. Covers nodes, users, comments, and taxonomy terms.
- Modifies certain basic CTools functions. Calls to ctools_include() are commented out; the object caching functions are rewritten to use the TempStore API; in some circumstances, it will try to convert ctools_get_plugins() to an empty plugin service.
- Updated routing access checks for compatibility with 8.0.x (added simple AccessResultInterface::allowedIf() wrapper around boolean return values in custom access callbacks)
- Grep converter type now uses YAML instead of JSON for easier editing; it's also a config file.
- Non-destructive. Most old code is left in place instead of being removed.

On the topic of QA, the next sprint will focus on automated testing of the current code, so that future refactorings are less likely to introduce regressions. Hope to see some of you in Amsterdam next week at:

• Drupal Module Upgrader BoF session Wed @ 15:45 - 16:45 in G111 - Adyax
• Upgrade YOUR Module to Drupal 8 Lab Thurs @ 13:00 - 15:15 in G103 - Rackspace

Woohoo!

Good idea! #2343137: Provide opt-in support for "destroy the old crap"

Crashing for now, but as an update, we had another demo call today with other OCTO folks, and here was the hit-list. Unfortunately I will not have a chance to go and issue-ize all of this, but so long as it's captured I figure that's better than nothing. :)

BEFORE AMSTERDAM:
----------------
- l()/url() use: how to match a D7 path to D8 path? can we use the routing system to resolve? if yes (and is literal), convert it, if no, @fixme.
  ** Internal URLs (admin/foo - convert these where possible)
  - Internal unrouted paths (robots.txt, cron.php) - router won't find it
  - External URLs (https://drupal.org/node/foo) - comment out
- access param should get AccountInterface $account variable in there somewhere. :)
- @fixme for unit tests
- Move any tests to the Tests namespace, even if we don't understand their base class.
- (During) Tag 0.5 after b1, if that happens during DrupalCon (we hope!)

IF POSSIBLE:
-----------
- changeAcces: instead of ->allowedIf() use ->allowedIfHasPermission()
- Fix AjaxTestCase and other base classes (create a map of names before/after?)
- --destroy flag (or --dry-run?)
Docblocks on generated methods
'HookMenuAlter' 'error'?
- Human readable name of test? How to get that instead of the class namespace?
- Tests change from ending in *Case to not. (PantsConfigurationTestCase => PantsConfigurationTest)
- $account / $user switched meanings between D7 => D8. ;( Can we switch them?

AFTER AMSTERDAM:
---------------
CTools plugins: See if there are patterns within popular contrib modules, try and do that use case. — start with plugins for ctools-dependent API modules (Views, Context, Entity Reference, Rules, Features?) - Ask eclipsegc for a recommemdation. (Drupal Commerce / Address Field)
- include file per plugin, starts with $plugin variable, move to annotation.
- functions that can be moved as methods to class
- class could be in that same file or could not be. :D

- What about Views?
  - Views handlers
  - Views plugins
  - hook_views_data()
  - BADCamp?
xjm to post a picture from a flip chart

Greetings! Long time no ping!

Back from DrupalCon Amsterdam, where our BoF was sparsely attended but our Upgrading Modules lab where we demoed DMU was *hugely* attended, so we managed to grow the number of participants in the issue queue by half a dozen or so! (Hello there, new folks! :D)

Over the last week and a half, phenaproxima has been working on refactoring the code to reduce stateful-ness and increase unit testability so that we could start adding some much-needed automated tests. However, this left a few bugs in the functionality which we are currently sorting out, with a tentative plan to release a 0.5 version on Wednesday or so.

Next up will be entity/field API stuff now that D8 is in beta and those APIs are more locked down. Adam also wants to embark on some serious refactoring to de-couple the code more now that we have a basic safety net of tests to inform us if things broke.

Ok, heeeeere's v0.6! https://www.drupal.org/node/2361937

By far the biggest change is a re-write on top of a new architecture which is hopefully much more approachable for new contributors. The DMU plugins are now split into four main types:

Analyzers

These plugins will add things to fix to the dmu-analyze report, along with links to docs on how to fix them.

Converters

These plugins actually convert the code from D7 to D8 (e.g. .info -> .info.yml).

Cleaners

After conversion, these plugins will clean out the old D7 code that's no longer needed (for example, delete .info files once .info.yml exists)

Indexers

These plugins (which are relatively few in number and only for modules that create new "architectural concepts") actually find the things in the code to analyze/convert (e.g. hook implementations, automated tests, etc.)

This separation is very nice because Analyzers are actually incredibly easy to write, and we can then start "crowd-sourcing" coverage of most of the changes in https://www.drupal.org/list-changes shortly, with Converter and Cleaner logic to be implemented later when/if someone has time.

Adam is in the process of writing extensive docs for this over at https://www.drupal.org/documentation/modules/drupalmoduleupgrader/contri.... Feedback wanted!

Another thing we could use feedback on is #2340935: Order analyze report into logical sections. Given that the dmu-analyze report will be indispensable for developers initially moving from D7 to D8, what's the most logical way in which to order the issues in the report?

Please give 0.6 a try and let us know what you think! :D

Ok, 1.0 of Drupal Module Upgrader has been tagged!! :) Grab it here: https://www.drupal.org/node/2374897

1.0 marks a feature-complete (analyze / upgrade / clean) and API stable (for plugin implementers) version.

Adam's internship at Acquia also ends tomorrow. Job super well done!! Both of us definitely plan to stay involved in this project though, on a volunteer basis. :)

But, since we will no longer be using this battleplan, and instead just using the normal issue queue going forward, going to close this issue out.

And remember: it's not goodbye, it's....