Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stats: Remove unused stats-lists from site page. #10629

Merged
merged 1 commit into from Jan 16, 2017

Conversation

timmyc
Copy link
Contributor

@timmyc timmyc commented Jan 13, 2017

There are times when I read old code and have moments of "what the .... is that doing there?" - and this is one of them 馃挴

Stroll Down Memory Lane
At one time, prior to the Insights tab being launched, Tags, Comments, and Followers ( aka nonPeriodicModules ) lived on the Stats Day/Week/Month/Year pages. When Insights was launched, these modules were still shown on the Stats "period" pages for a while - I believe while A/B testing - but have long since permanently moved to Insights.

But yet these lines of code have remained well after their welcome was long gone... almost like a random friend from college who wanted to crash at your house for a week, but is still there two months later.

Well, old stats lists, it is time you are shown to the door... and take your five un-needed network requests with you.

To Test

  • Open site stats pages and test out Day, Week, Month, Year. Verify all works well after the old stats lists have been removed.
  • Open the Insights tab. Verify all is well there too.

@timmyc timmyc added Stats Everything related to our analytics product at /stats/ [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Janitorial labels Jan 13, 2017
@matticbot
Copy link
Contributor

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such an awesome PR! :) 馃殺

@youknowriad youknowriad added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 16, 2017
@timmyc timmyc force-pushed the update/stats/remove-unused-lists branch from b1994d6 to 8ce6d33 Compare January 16, 2017 23:13
@timmyc timmyc merged commit 206f377 into master Jan 16, 2017
@timmyc timmyc deleted the update/stats/remove-unused-lists branch January 16, 2017 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stats Everything related to our analytics product at /stats/ [Type] Janitorial
Projects
No open projects
Stats
馃殺
Development

Successfully merging this pull request may close these issues.

None yet

4 participants