Page MenuHomePhabricator

Refactor Linter config values into one method getLinterConfig()
Closed, ResolvedPublic

Description

During triage, it was determined that the current status of linter config variables is acceptable and new refinements or smashing of variables will be determined as needed.

Summary:

While the web team collaboration with the content transformation team on one of the tasks T336528, we've recognised an exciting opportunity to enhance code quality and maintainability. Our current linting rule configurations are spread across various methods and maybe files, which can lead to confusion. To tackle this, we propose implementing a fantastic method called getLinterConfig in our codebase. This method will provide configuration values for each linting rule, offering a centralized and efficient approach.

Use Cases:

  1. Centralized Configuration: By introducing the getLinterConfig method, we can conveniently manage and modify linting rule configurations from a single point of access.
  2. Collaborative Efforts: Working closely with the content transformation team, we ensure a cohesive and united approach towards improving linting rules together.
  3. Modularity: Embracing the centralised provider will make it a breeze to add new linting rules without disrupting other parts of the codebase.

Benefits:

  1. Improved Code Quality: The getLinterConfig method will empower us to uphold consistent and standardized coding practices, raising the overall code quality to new heights.
  2. Enhanced Productivity: Embracing this centralised approach streamlines the process of updating linting rules, saving valuable development time and effort.

Acceptance Criteria:

  • The getLinterConfig method should be implemented in the codebase, bringing us closer to our goal of better linting rule configurations.
  • It must effectively return configuration values for all existing linting rules, ensuring a robust solution.
  • It must have the ability to return partial config per linting rule.
  • Write comprehensive unit tests to ensure the correctness of the getLinterConfig method, making sure it delivers reliable results.
  • Collect feedback from both the web and content transformation teams, documents notes for future iterations.

Event Timeline

Change 944945 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/services/parsoid@master] Introduce SiteConfig::getLintConfig() and pull in all linter configuration

https://gerrit.wikimedia.org/r/944945

Sorry, I did a quick first draft of this, patch above. It still needs some work, though, so happy to hand it off:

  • no test suite!
  • not actually hooked up in core.

The latter is a bit tricky: the new configuration variable needs to be defined in the extension.json for the Linter extension -- but it's the implementation of SiteConfig::getLintConfig() in *core* which wants to return the values. And having core directly depend on an extension is frowned upon.

So I think you actually need to add a *hook* in core, which the Linter extension will then implement to return the value of its config var. And then of course it would be good to set some default values in mediawiki-config to demonstrate that we can actually turn on/off different lints and adjust parameters etc. So *four* patches altogether to different repos:

  • the patch above to Parsoid, which introduces SiteConfig::getLintConfig()
  • a patch to core, which calls a 'GetLinterConfig' hook and returns it's value, or if no hook is registered (or the hook returns null?) returns the default values from parent::getLintConfig()
    • Could be done in a number of ways, but we should provide a way for the hook to pass through default values from parent::getLintConfig() and only add to them.
  • a patch to the Linter extension, which defines a new wgParsoidLintConfig (or whatever) configuration variable, and implements the GetLinterConfig hook to return $defaultValue + $configValue
  • a patch to the mediawiki-config repo, which sets default values for wgParsoidLintConfig per-wiki.

Change 944945 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Introduce SiteConfig::getLinterConfig() and pull in all linter configuration

https://gerrit.wikimedia.org/r/944945

Change 982166 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.19.0-a8

https://gerrit.wikimedia.org/r/982166

Change 982166 merged by jenkins-bot:

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.19.0-a8

https://gerrit.wikimedia.org/r/982166

Change 993054 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/services/parsoid@master] Remove deprecated SiteConfig (linter related) methods

https://gerrit.wikimedia.org/r/993054

Change 993054 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Remove deprecated SiteConfig (linter related) methods

https://gerrit.wikimedia.org/r/993054

Sbailey updated the task description. (Show Details)