Page MenuHomePhabricator

Feature branch: Remove pageIssues A-B test code
Closed, ResolvedPublic5 Estimated 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.

Related Objects

StatusSubtypeAssignedTask
OpenNone
StalledNone
OpenNone
DeclinedNone
DuplicateJdlrobson
OpenNone
OpenNone
DeclinedNone
OpenNone
ResolvedJdlrobson
Resolvedovasileva
ResolvedNiedzielski
DuplicateNone
Openmarcella
OpenNone
OpenNone
OpenNone
OpenNone
Resolvedmatmarex
ResolvedNone

Event Timeline

Jdlrobson triaged this task as Medium 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.

All JS relating to instrumentation is removed

Does this include the changes to load ReadingDepth after Minerva?

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).

> 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 Medium 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 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...

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

nray removed nray as the assignee of this task.
nray added a subscriber: nray.