Problem/Motivation

I tried to to run upgrade_status:checkstyle command for all my custom modules (./vendor/bin/drush upgrade_status:checkstyle --all --ignore-contrib > checkstyle.xml).

I expected a XML with with all errors/warning, but the command output contains multiple xml headers and root elements. The result is not a valid xml file.

Steps to reproduce

./vendor/bin/drush upgrade_status:checkstyle --all --ignore-contrib > checkstyle.xml

Proposed resolution

  • Untangle the current logic in upgrade_status:analyze
  • Create an array of Result (?) object with can be used for the various output formats: text, xml

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#5 3171307-5.patch6.21 KBwebflo
#3 3171307-13.patch5.95 KBwebflo

Comments

webflo created an issue. See original summary.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: upgrade_status:checkstyle incompatible with multiple projects » upgrade_status:checkstyle does not produce valid XML output when multiple projects are scanned at once

It took me a minute to realize what the problem is. Retitled for that. We are running this checkstyle thing on all 9k+ projects at https://dispatcher.drupalci.org/job/project_analysis/ so it does work fine for almost all of contrib one by one. This is about running it on multiple projects AT ONCE.

webflo’s picture

webflo commented
Status: Active » Needs review
StatusFileSize
new 3171307-13.patch5.95 KB

Thanks for the better issue title :)

I realized most of the logic is already separated, i moved the integration with deprecationAnalyzer to ::doAnalyze. analyze and checkstyle handle the output format only.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

Great refactor. Thanks! Almost good to commit IMHO.

+++ b/src/Commands/UpgradeStatusCommands.php
@@ -93,8 +93,40 @@ class UpgradeStatusCommands extends DrushCommands {
-    $this->mode = 'checkstyle';

This makes the mode property unused and that should also be removed.

webflo’s picture

webflo commented
Status: Needs work » Needs review
StatusFileSize
new 3171307-5.patch6.21 KB

Right. I removed the property and changed the doAnalyze method to public. Its a useful method for contrib to support additional output formats.

  • Gábor Hojtsy committed 9fda895 on 8.x-3.x 
    Issue #3171307 by webflo: upgrade_status:checkstyle does not produce...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Fixed

Committed, thanks a lot!

Status: Fixed » Closed (fixed)

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