Skip to content

Statistics code clean up #600

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 15, 2021
Merged

Statistics code clean up #600

merged 5 commits into from
Mar 15, 2021

Conversation

markets
Copy link
Collaborator

@markets markets commented Mar 13, 2021

This code needs further refactoring (probably move all logic to Services/Decorators), this is just a 1st minor step cleaning some details here and there and adding some tests.

SUMMARY OF CHANGES

  • remove redundant and too much verbose statistics_ prefix in files, methods, keys, routes, ...
  • remove unnecessary and confusing @Ivars
  • remove useless comments
  • rename some vars (ini/fi -> from/to)
  • some code re-arrange and re-grouping (NO logic changes)
  • improved test coverage ⬆️

NOTES

EXTRAS
In the Admin section, add a "Recent Posts" box, inline with "Recent users" and "Recent Orgs" (489dab6)

REVIEW LINK (with no whitespace changes)
https://github.com/coopdevs/timeoverflow/pull/600/files?diff=unified&w=1

QA
Business logic was not changed at all, so a quick visit on each stats page to see everything keeps displaying data is enough :)

@markets markets added the wip label Mar 13, 2021
@markets markets force-pushed the refactor_statistics branch from feb7fd0 to e4b6280 Compare March 13, 2021 20:34
@sseerrggii
Copy link
Contributor

Tested!! 🌵

@sseerrggii sseerrggii merged commit d2c3ba5 into develop Mar 15, 2021
@markets markets deleted the refactor_statistics branch March 15, 2021 10:27
@sseerrggii
Copy link
Contributor

@markets
Copy link
Collaborator Author

markets commented Mar 15, 2021

@sseerrggii
Copy link
Contributor

deleted!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants