- identify the pain points of mobilefrontend in terms of difficulty, efficiency, and ease of building and maintaining features from their perspective
- brainstorm optimal solutions
- provide result to whomever T156259: [Spike 1hr] What technical problems do we need to fix from mobilefrontend - ENG is assigned to
Description
Description
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Jdlrobson | T156257 Strategy Research - Changing mobilefrontend | |||
| Resolved | None | T156259 [Spike 1hr] What technical problems do we need to fix from mobilefrontend - ENG | |||
| Resolved | pmiazga | T157128 [Spike 1hr] What technical problems do we need to fix from mobilefrontend |
Event Timeline
Comment Actions
PHPMessDetector complains a lot
raynor@DellE6540:~/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes(master○) » phpmd . text codesize,cleancode,design | grep -v 'Avoid using static' | grep -v 'uses an else' /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MenuBuilder.php:43 The method insert has a boolean flag argument $isJSOnly, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MenuBuilder.php:81 The method insertAfter has a boolean flag argument $isJSOnly, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13 The class MobileContext has 1240 lines of code. Current threshold is 1000. Avoid really long classes. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13 The class MobileContext has 49 public methods and attributes. Consider to reduce the number of public items under 45. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13 The class MobileContext has 21 fields. Consider to redesign MobileContext to keep the number of fields under 15. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13 The class MobileContext has 33 non-getter- and setter-methods. Consider refactoring MobileContext to keep number of methods under 25. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13 The class MobileContext has 23 public methods. Consider refactoring MobileContext to keep number of public methods under 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13 The class MobileContext has an overall complexity of 167 which is very high. The configured complexity threshold is 50. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13 The class MobileContext has a coupling between objects value of 13. Consider to reduce the number of dependencies under 13. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:812 The method getMobileUrl has a boolean flag argument $forceHttps, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:11 The class MobileFormatter has an overall complexity of 89 which is very high. The configured complexity threshold is 50. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:115 The method enableTOCPlaceholder has a boolean flag argument $flag, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:124 The method enableExpandableSections has a boolean flag argument $flag, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:132 The method setIsMainPage has a boolean flag argument $value, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:146 The method filterContent has a boolean flag argument $removeDefaults, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:146 The method filterContent has a boolean flag argument $removeReferences, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:146 The method filterContent has a boolean flag argument $removeImages, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:147 The method filterContent has a boolean flag argument $showFirstParagraphBeforeInfobox, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:458 The method parseMainPage() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:458 The method parseMainPage() has an NPath complexity of 208. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:541 The method makeSections() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.body.php:34 The method DOMParse() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.body.php:34 The method DOMParse has a boolean flag argument $isBeta, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.body.php:99 The method getUserPageContent() has an NPath complexity of 625. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:17 The class MobileFrontendHooks has 1412 lines of code. Current threshold is 1000. Avoid really long classes. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:17 The class MobileFrontendHooks has 44 non-getter- and setter-methods. Consider refactoring MobileFrontendHooks to keep number of methods under 25. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:17 The class MobileFrontendHooks has 41 public methods. Consider refactoring MobileFrontendHooks to keep number of public methods under 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:17 The class MobileFrontendHooks has an overall complexity of 173 which is very high. The configured complexity threshold is 50. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:17 The class MobileFrontendHooks has a coupling between objects value of 23. Consider to reduce the number of dependencies under 13. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:186 The method onOutputPageBeforeHTML() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:591 The method onSpecialPageBeforeExecute() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:723 The method onBeforePageDisplay() has a Cyclomatic Complexity of 26. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:723 The method onBeforePageDisplay() has an NPath complexity of 186624. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:723 The method onBeforePageDisplay() has 128 lines of code. Current threshold is set to 100. Avoid really long methods. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.skin.hooks.php:311 The method getSitename has a boolean flag argument $withPossibleTrademark, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:10 The class ApiMobileView has an overall complexity of 156 which is very high. The configured complexity threshold is 50. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:10 The class ApiMobileView has a coupling between objects value of 20. Consider to reduce the number of dependencies under 13. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:49 The method execute() has a Cyclomatic Complexity of 48. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:49 The method execute() has an NPath complexity of 6252134400. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:49 The method execute() has 210 lines of code. Current threshold is set to 100. Avoid really long methods. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:387 The method getRequestedSectionIds() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:487 The method parseSectionsData has a boolean flag argument $useTidy, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:529 The method getData() has a Cyclomatic Complexity of 25. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:529 The method getData() has an NPath complexity of 179200. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:529 The method getData() has 148 lines of code. Current threshold is set to 100. Avoid really long methods. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:712 The method addPageImage() has a Cyclomatic Complexity of 22. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:712 The method addPageImage() has an NPath complexity of 188280. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/models/MobilePage.php:45 The method __construct has a boolean flag argument $file, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/models/MobilePage.php:147 The method getSmallThumbnailHtml has a boolean flag argument $useBackgroundImage, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/models/MobilePage.php:158 The method getPageImageHtml has a boolean flag argument $useBackgroundImage, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/MinervaTemplate.php:231 The method getPreContentHtml() has an NPath complexity of 625. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:13 The class SkinMinerva has 1379 lines of code. Current threshold is 1000. Avoid really long classes. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:13 The class SkinMinerva has 31 non-getter- and setter-methods. Consider refactoring SkinMinerva to keep number of methods under 25. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:13 The class SkinMinerva has an overall complexity of 179 which is very high. The configured complexity threshold is 50. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:13 The class SkinMinerva has a coupling between objects value of 23. Consider to reduce the number of dependencies under 13. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:216 The method doEditSectionLink has a boolean flag argument $lang, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:1244 The method getContextSpecificModules() has a Cyclomatic Complexity of 16. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:1244 The method getContextSpecificModules() has an NPath complexity of 1800. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/MobileSpecialPageFeed.php:86 The method renderFeedItemHtml() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/MobileSpecialPageFeed.php:86 The method renderFeedItemHtml() has an NPath complexity of 288. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/MobileSpecialPageFeed.php:87 The method renderFeedItemHtml has a boolean flag argument $isAnon, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/MobileSpecialPageFeed.php:87 The method renderFeedItemHtml has a boolean flag argument $isMinor, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:9 The class SpecialMobileDiff has an overall complexity of 56 which is very high. The configured complexity threshold is 50. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:59 The method getRevisionsToCompare() has a Cyclomatic Complexity of 12. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:220 The method showDiff() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:220 The method showDiff() has an NPath complexity of 1440. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:380 The method getMobileUrlFromDesktop() has a Cyclomatic Complexity of 14. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:380 The method getMobileUrlFromDesktop() has an NPath complexity of 918. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileHistory.php:180 The method showRow() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileLanguages.php:136 The method executeWhenAvailable() has a Cyclomatic Complexity of 12. The configured cyclomatic complexity threshold is 10. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileOptions.php:89 The method addSettingsForm() has an NPath complexity of 2000. The configured NPath complexity threshold is 200. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileOptions.php:296 The method getURL has a boolean flag argument $fullUrl, which is a certain sign of a Single Responsibility Principle violation. /home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileWatchlist.php:11 The class SpecialMobileWatchlist has a coupling between objects value of 14. Consider to reduce the number of dependencies under 13
Comment Actions
PHPStorm inspection also complains a lot:
- 9 deprecated calls
- 6 incosistent return points
- 4 parameters mismatch in declaration
- ~40 unsued variables/parameters
- 12 Type combaility issues
- 74 undefined issues
Type compability issues and undefined issues might be false positive
Comment Actions
Phan also complains but not that much:
extensions/MobileFrontend/includes/MobileFrontend.hooks.php:555 PhanUndeclaredTypeParameter Parameter of undeclared type \AbuseFilterVariableHolder extensions/MobileFrontend/includes/MobileFrontend.hooks.php:1001 PhanUndeclaredTypeParameter Parameter of undeclared type \CentralAuthUser extensions/MobileFrontend/includes/MobileFrontend.hooks.php:1020 PhanUndeclaredTypeParameter Parameter of undeclared type \CentralAuthUser extensions/MobileFrontend/includes/diff/InlineDifferenceEngine.php:121 PhanUndeclaredFunction Call to undeclared function \wikidiff2_inline_diff() extensions/MobileFrontend/tests/phpunit/devices/CustomHeaderDeviceDetectorTest.php:18 PhanUndeclaredTypeProperty Property \Tests\MobileFrontend\Devices\CustomHeaderDeviceDetectorTest::config has undeclared type \Tests\MobileFrontend\Devices\GlobalVarConfig extensions/MobileFrontend/tests/phpunit/devices/CustomHeaderDeviceDetectorTest.php:23 PhanUndeclaredTypeProperty Property \Tests\MobileFrontend\Devices\CustomHeaderDeviceDetectorTest::detector has undeclared type \Tests\MobileFrontend\Devices\UADeviceDetector
Comment Actions
Things to improve:
- high code complexity, long functions, even longer classes
- lots of coupling with MediaWiki Core, extension changes MW behaviour bit too much
- time for SOA ?
- MobileFrontend - is an extension or is it a Skin? - this extension handles so much it could be break into 2-3 separate extensions
- missing tests (30% code coverage is not enough)
- on-fly html modifications are error-prone, also very slow
- scaffolding for ongoing experiments/beta features
- extension contains logic required only on Wikimedia cluster (not 3rd party instansces)