Re: [Wikitech-l] MediaWiki Technical Debt
On Fri, Oct 24, 2014 at 3:37 PM, Addshore wrote: > I did some work toward getting scrutinizer some time ago including the > following pull req that has been merged (and I guess deployed now) as prior > to this the inspection crashed out. > https://github.com/scrutinizer-ci/php-analyzer/pull/133 > Sadly this php-analyzer project is no longer open source(doesn't explicitly say so, but its master branch is blanked), so in the future we can't fix these problems. ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] MediaWiki Technical Debt
The maximum allowed limit is 5000 issues. We currently only have 3886 (or thats what the scan is currently showing... It would be great to get the .scrutinizer.yml file in the git repo so that we can see what inspections are actually being run and people can poke them! I did some work toward getting scrutinizer some time ago including the following pull req that has been merged (and I guess deployed now) as prior to this the inspection crashed out. https://github.com/scrutinizer-ci/php-analyzer/pull/133 Great work Hashar! On 23 October 2014 23:07, Jeroen De Dauw wrote: > Hey, > > > Since a friend introduced me to Scrutinizer yesterday and the graph above > seems to be based on it, > > SensioLabsInsight != ScrutinizerCI > > I added mediawiki/core to their interface: > > > > https://scrutinizer-ci.com/g/wikimedia/mediawiki/ > > > > Yay. I tried doing this a year ago or so, and back at that point the > analysis just aborted due to too many issues. Guess the limit was raised :) > > That being said, is there any point in fixing all those "issues"? And if > > so how do we track them and make sure they are not reintroduced with new > > patchsets? > > > > Going though the issue list and getting rid of all the warnings is probably > not a good use of your time. Going though and seeing if it points you to > something pressing might be worthwhile. What I personally find very > valuable is that you can get a list of classes sorted by complexity, or by > coupling, or by quality rating, to get an idea of what areas of the > codebase could use some love [0]. > > At Wikimedia Deutchland we use ScrutinizerCI for most of our PHP > components, and have it run for each commit merged into master. You can > then see the changes per commit [1], get weekly reports [2] and view the > overall trend [3]. > > [0] > https://scrutinizer-ci.com/g/wmde/WikibaseDataModel/code-structure/master > [1] > > https://scrutinizer-ci.com/g/wmde/WikibaseDataModel/inspections/07fab814-f6bc-42df-aab4-80f745d7f0d9 > [2] https://scrutinizer-ci.com/g/wmde/WikibaseDataModel/reports/ > [3] https://scrutinizer-ci.com/g/wmde/WikibaseDataModel/statistics/ > > Cheers > > -- > Jeroen De Dauw - http://www.bn2vs.com > Software craftsmanship advocate > Evil software architect at Wikimedia Germany > ~=[,,_,,]:3 > ___ > Wikitech-l mailing list > Wikitech-l@lists.wikimedia.org > https://lists.wikimedia.org/mailman/listinfo/wikitech-l > -- Addshore ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] MediaWiki Technical Debt
Hey, > Since a friend introduced me to Scrutinizer yesterday and the graph above seems to be based on it, SensioLabsInsight != ScrutinizerCI I added mediawiki/core to their interface: > > https://scrutinizer-ci.com/g/wikimedia/mediawiki/ > Yay. I tried doing this a year ago or so, and back at that point the analysis just aborted due to too many issues. Guess the limit was raised :) That being said, is there any point in fixing all those "issues"? And if > so how do we track them and make sure they are not reintroduced with new > patchsets? > Going though the issue list and getting rid of all the warnings is probably not a good use of your time. Going though and seeing if it points you to something pressing might be worthwhile. What I personally find very valuable is that you can get a list of classes sorted by complexity, or by coupling, or by quality rating, to get an idea of what areas of the codebase could use some love [0]. At Wikimedia Deutchland we use ScrutinizerCI for most of our PHP components, and have it run for each commit merged into master. You can then see the changes per commit [1], get weekly reports [2] and view the overall trend [3]. [0] https://scrutinizer-ci.com/g/wmde/WikibaseDataModel/code-structure/master [1] https://scrutinizer-ci.com/g/wmde/WikibaseDataModel/inspections/07fab814-f6bc-42df-aab4-80f745d7f0d9 [2] https://scrutinizer-ci.com/g/wmde/WikibaseDataModel/reports/ [3] https://scrutinizer-ci.com/g/wmde/WikibaseDataModel/statistics/ Cheers -- Jeroen De Dauw - http://www.bn2vs.com Software craftsmanship advocate Evil software architect at Wikimedia Germany ~=[,,_,,]:3 ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] MediaWiki Technical Debt
> > Some more examples that might be useful in some form: > - Global variable or function should never be used > (This is probably the only really useful one, because removing global > functions/variables would result in better testable code.) > - PHP debug statements found In maintenance scripts and things like eval.php that's fine. Using it in the debug api format also seems fine imo. > - Logical operators should be avoided I'll give it that one as legitimate. > - sleep() should not be used Its reasonable in a maintenance script which is what was flagged. > - Source code should not contain FIXME comments Better than just not documenting things needing to be fixed. (However, arguably the report is listing stuff that should be fixed, so its appropriate to list that, I just don't like the shoot-the-messenger way its phrased. Code marked FIXME is better than the same code with no indication it needs to be fixed.) Also the time estimates to fix issues that it flagged (supposing they are legit issues), seem rather high. --bawolff ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] MediaWiki Technical Debt
Le 23/10/2014 14:46, Tyler Romeo a écrit : > Found this today: https://twitter.com/symfony_en/status/525222757567827968 > > It's probably a useless statistic, but I still found it amusing. Good to > know we still have less technical debt than WordPress. ;) Hello, Since a friend introduced me to Scrutinizer yesterday and the graph above seems to be based on it, I added mediawiki/core to their interface: https://scrutinizer-ci.com/g/wikimedia/mediawiki/ I might have configured it to run once per day against the master branch and also sent invitations to a bunch of folks being admin on github. That being said, is there any point in fixing all those "issues"? And if so how do we track them and make sure they are not reintroduced with new patchsets? -- Antoine "hashar" Musso ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] MediaWiki Technical Debt
Actual results for mediawiki core: https://insight.sensiolabs.com/projects/8b5d527f-f70c-4fc0-977f-de99743eae06/analyses/16 Over all I think making stuff more testable with phpunit is much more important than going through any parts of that report. On 2014-10-23 17:16, Jeroen De Dauw wrote: >> It's probably a useless statistic > > It's based on SensioLabsInsight, which I personally quite dislike. > ScrutinizerCI is hands down better at spotting issues (way to many false > positives on SensioLabsInsight). I think PHP_CodeSniffer (possibly with a few new sniffs) can replace all of their tests. There are many important metrics they do not consider at all. Something quite better than what they have can be obtained with the following tools: phpunit (code coverage), phpcpd (copied code), warnings from your documentation generator like phpdox, PHP_Depend, phploc, PHPMD, PHP_CodeSniffer. See http://jenkins-php.org for how to integrate that into our CI infrastructure. Yes, many false positives. Many of their tests are only of the type: search for where $function is called. In fact I think none of their tests can be classified as static analysis (not sure about "Database queries should use parameter binding", though). Many of their tests can only be used as hints because there are enough instances where that is exactly what one needs to do. Examples: - exit() and die() functions should be avoided - PHP configuration should not be changed dynamically Some more examples that might be useful in some form: - Global variable or function should never be used (This is probably the only really useful one, because removing global functions/variables would result in better testable code.) - PHP debug statements found - Logical operators should be avoided - sleep() should not be used - Source code should not contain FIXME comments -- Regards, Jan Zerebecki ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] MediaWiki Technical Debt
Much agreed on this. I tried out SensioLabsInsight once or twice, and overall it has never given me anything useful that PhpStorm’s built-in inspections did not already catch. -- Tyler Romeo 0x405D34A7C86B42DF On October 23, 2014 at 11:17:22, Jeroen De Dauw (jeroended...@gmail.com) wrote: It's based on SensioLabsInsight, which I personally quite dislike. ScrutinizerCI is hands down better at spotting issues (way to many false positives on SensioLabsInsight). ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] MediaWiki Technical Debt
On 23 October 2014 16:16, Jeroen De Dauw wrote: > I've looked at the code and it's quality of most of these projects at some > point in the last two years, and have to say this graph is much in line > with my impressions. And with what the "knowledgeable" people at PHP > conferences and meetups convey. Want an example of bad code? Wordpress! > Want one of good code? Symfony or Doctrine! Sadly enough MediaWiki seems to > be entirely absent as a topic or even something to rant about at such > meetups and conferences. Wonder how Magento would do. They might have to go to log scale on the Y axis. - d. ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Re: [Wikitech-l] MediaWiki Technical Debt
Hey, I've looked at the code and it's quality of most of these projects at some point in the last two years, and have to say this graph is much in line with my impressions. And with what the "knowledgeable" people at PHP conferences and meetups convey. Want an example of bad code? Wordpress! Want one of good code? Symfony or Doctrine! Sadly enough MediaWiki seems to be entirely absent as a topic or even something to rant about at such meetups and conferences. > It's probably a useless statistic It's based on SensioLabsInsight, which I personally quite dislike. ScrutinizerCI is hands down better at spotting issues (way to many false positives on SensioLabsInsight). > Good to know we still have less technical debt than WordPress. ;) Perhaps not the best standard to set for oneself :) Cheers -- Jeroen De Dauw - http://www.bn2vs.com Software craftsmanship advocate Evil software architect at Wikimedia Germany ~=[,,_,,]:3 ___ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
