-
Notifications
You must be signed in to change notification settings - Fork 40
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
Remove statistics module #22
Comments
not quite sure how to do this but: backdrop/backdrop#36 |
Hi @alexweber, thanks for this pull request! Great work referencing the pull request. You're right that it's a little weird having a separate issue tracker, but due to Github's permissions requirements (labels on issues requires push access). Thanks for being an early contributor! :) |
Hey @quicksketch my pleasure, kinda curious as to where this thing will go... :) |
@timmillwood lol true that man, it just needs some ajax love and everything will be fine :) |
Thanks @alexweber! As @justafish has pointed out though, this isn't passing tests (though Travis CI thinks it is: https://travis-ci.org/backdrop/backdrop/builds/11086512). Now something has conflicted with it too (we're removing a lot of stuff after all). Could you submit a new PR? For extra kudos, reference this issue in all of your commit messages. A la "Issue #22: Remove statistics module." This isn't a big deal yet while we're removing things, but as we start adding new features, we want the "git blame" command to be able to associate each line number with an issue. |
@quicksketch for sure man, I'll have it ready in a few hours! |
@quicksketch So the plot thickens, search.module uses statistics.module for view rankings, I'm assuming we're gonna rip that out too, feedback welcome. |
Yep, take it out. :) The dependency between Search and Statistics is soft afaik. Search should continue to function fine without statistics, since most sites don't enable statistics to begin with. Keep on deleting! |
Yup, the only thing tying them together was a few lines in search.test, PR incoming |
Merged. Great work! |
Sweet! |
sub-issue from #7
The text was updated successfully, but these errors were encountered: