Page MenuHomePhabricator

Feature branch: Remove pageIssues A-B test code
Closed, ResolvedPublic5 Story Points

Description

As soon as the page issues code is shipped we should remove the code relating to instrumentation - it adds unnecessary bloat to our codebase and has a high maintenance cost.

Acceptance criteria

  • Changes should be made in a feature branch called "page-issues-cleanup". Use git review page-issues-cleanup to post code there.
  • Code should be removed in a single patchset, to make it easier to restore this in future if needed.
  • All JS relating to instrumentation in Minerva is removed (no changes to ReadingDepth should be necessary)
  • All PHP relating to config is removed
  • Cleanup any production config
  • there should be no flash of unstyled content for the new treatment.

Developer notes

Code to remove includes:

    • pageIssues.js **
  • Remove formatPageIssuesSeverity
  • abTest
  • References to pageIssuesLogger removed
  • isLoggingRequired removed
  • removed all pageIssuesLogger.log calls
    • pageIssuesLogger.js **
  • Removed in full

PageIssuesOverlay

  • PageIssuesOverlay no longer takes a PageIssuesLogger
  • PageIssuesOverlay.prototype.events, log, onEditLink, onInternalLink, onRedLinkClick, onExit code can be removed.
  • issuesSeverity and sectionNumbers no longer needs to be sent as part of modalClose event
  • parseSeverity and issueSummaryToSeverity helpers removed

Sign off step

  • Make sure there is a task open to merge the feature branch to master.

Details

Related Gerrit Patches:
mediawiki/skins/MinervaNeue : page-issues-cleanupRemove page issues instrumentation

Related Objects

StatusAssignedTask
OpenNone
OpenNone
OpenNone
OpenJdlrobson
OpenNone
OpenNone
DeclinedNone
OpenNone
ResolvedJdlrobson
Resolvedovasileva
ResolvedNiedzielski
ResolvedDannyH
OpenNone
DuplicateNone
Openmarcella
OpenNone
OpenNone
OpenNone
Resolvedmatmarex
ResolvedNone

Event Timeline

Jdlrobson triaged this task as Normal priority.Oct 3 2018, 10:22 PM
Jdlrobson created this task.
Jdlrobson moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.
Jdlrobson updated the task description. (Show Details)Oct 4 2018, 2:35 PM

All JS relating to instrumentation is removed

Does this include the changes to load ReadingDepth after Minerva?

Jdlrobson updated the task description. (Show Details)Oct 4 2018, 3:06 PM

Sorry (just reviewing the code), should we also delete the query parameter override logic?

Sorry (just reviewing the code), should we also delete the query parameter override logic?

Depends on whether we turn this on for all projects (see T206179).

phuedx removed a subscriber: phuedx.Oct 4 2018, 4:39 PM
Quiddity removed a subscriber: Quiddity.Oct 4 2018, 10:34 PM

> there should be no flash of unstyled content for the new treatment.
If we only need to support one treatment, we know up front how to style the ambox (currently hidden by default). We can display it and avoid the pushing down of content

ovasileva set the point value for this task to 5.Oct 9 2018, 4:38 PM
Jdlrobson raised the priority of this task from Normal to High.Oct 9 2018, 4:39 PM
Jdlrobson moved this task from Triaged but Future to Upcoming on the Readers-Web-Backlog board.

If possible, please rename CSS class names from A/B-test specific to main/non-main namespace specific since both treatments must be kept.

Jdlrobson updated the task description. (Show Details)Oct 10 2018, 9:59 PM
Jdlrobson renamed this task from Remove pageIssues A-B test code to Feature branch: Remove pageIssues A-B test code.Oct 16 2018, 6:10 PM
Jdlrobson updated the task description. (Show Details)

Change 467842 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@page-issues-cleanup] Remove page issues instrumentation

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

Change 467842 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@page-issues-cleanup] Remove page issues instrumentation

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

skipping QA as this can be taken care of when we merge this back to master.
Needing a signer offer...

Jdlrobson removed Jdlrobson as the assignee of this task.Oct 24 2018, 11:00 PM
Jdlrobson assigned this task to nray.Oct 31 2018, 5:24 PM
nray updated the task description. (Show Details)Nov 1 2018, 4:38 PM

Per sign off instructions, created new task T208514 for merging feature branch to master

nray closed this task as Resolved.Nov 1 2018, 4:45 PM
nray removed nray as the assignee of this task.
nray added a subscriber: nray.
Elitre removed a subscriber: Elitre.Nov 8 2018, 5:01 PM
Nirmos removed a subscriber: Nirmos.Nov 12 2018, 10:46 PM