There's still tremendous value, especially for new Drupal 8 users, in having a pointer to the relevant change notices that affect their modules, but PHPCS's token parsing is too hard to work with.

The report that PHPCS generates looks like this:

FILE: /Users/webchick/Sites/8.x/modules/pants/pants.module
--------------------------------------------------------------------------------
FOUND 14 ERRORS AFFECTING 10 LINES
--------------------------------------------------------------------------------
  13 | ERROR | [ ] Routing functionality of hook_menu() has been replaced by
     |       |     new routing system, conditionals found, cannot change
     |       |     automatically: https://drupal.org/node/1800686
  58 | ERROR | [x] hook_form_user_profile_form_alter() was renamed:
     |       |     https://drupal.org/node/1734556
 101 | ERROR | [x] variable_get("type") has been replaced by the Configuration
     |       |     API: https://drupal.org/node/2183531
 101 | ERROR | [ ] variable_get("type") has been replaced by the Configuration
     |       |     API: https://drupal.org/node/2183531
 132 | ERROR | [x] Theme functions should be converted to Twig templates:
     |       |     https://drupal.org/node/1831138
 151 | ERROR | [x] Theme functions should be converted to Twig templates:
     |       |     https://drupal.org/node/1831138
 168 | ERROR | [x] Theme functions should be converted to Twig templates:
     |       |     https://drupal.org/node/1831138
 171 | ERROR | [x] variable_get("recent_block_count") has been replaced by the
     |       |     Configuration API: https://drupal.org/node/2183531
...

Our thinking was something more inverted, that starts with the change record and then enumerates what files/functions/line numbers(?) are affected.

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Add a --verbose option to dmu-upgrade to put link to change record above changes » Ditch PHPCodeSniffer, in favor of "dmu-analyze" report containing similar thing
Priority: Major » Critical

Re-titling/re-purposing.

We decided through the course of the call that PHPCodeSniffer no longer makes sense. Drush is way more widely deployed among the audience of this project, and then we don't force contributors to have to write two different versions of detection rules.

Also talked about as a result of this, splitting the rules between detect() and fix().

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Issue summary: View changes
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

I've removed all the PHPCS-related code (Atlas, put down your burden!) and have started refactoring the plugins to have analyze() and convert() methods.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Here's the current dmu-analyze output:

$ drush dmu-analyze pants
theme() has been renamed to _theme() in Drupal 8, and should never be[error]
called directly (use render arrays instead).
  http://drupal.org/node/2195739
variable_get() has been removed in Drupal 8.                         [error]
  http://drupal.org/node/2183531
hook_block_info() has been removed in Drupal 8, and blocks are now   [error]
plugins in the Plugin\Block namespace.
hook_menu() has been removed in Drupal 8 and replaced with a         [error]
completely new routing and linking system.
  http://drupal.org/node/1800686
Info file must contain a "core" value of "8.x".                      [error]
  http://drupal.org/node/1935708
Info file must contain a "type" value of "module".                   [error]
  http://drupal.org/node/1935708
Many modules have moved into core and no longer need to be declared  [warning]
as dependencies.
  http://drupal.org/node/1935708
Drupal core now includes a PSR-4 compatible autoloader, so the       [error]
files[] array is defunct.
  http://drupal.org/node/1935708
Info file "configure" key must be a route name, not a path.          [error]
  http://drupal.org/node/1935708
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Comments on cleaning up the output would be most welcome!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

If it's easy to do, one small request would be to preface the URLs with something like:

"API change record for more info: "

If that's not easy, then maybe a blurb of help at the top that explains what all of those links are about.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

It's not difficult (see ConverterBase::flag()). But should we hold off on that until we get more input from @xjm?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Yeah, perhaps. Just jotting things down as I think of them. :) Cool to know where to change it though.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Fixed
Related issues: +#2328647: dmu-analyze's output is hideous

Marking this fixed because dmu-analyze is implemented and working. It's ugly as sin, but I've opened up a separate issue for that.

Status: Fixed » Closed (fixed)

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