Prepare to accept custom Collectors #46
Conversation
| result = described_class.new(population: populate).call(&observable) | ||
|
|
||
| expect(result.size).to eq 2 | ||
| expect(result.first[0]).to eq 2 | ||
| expect(result.first[1].size).to eq 3 | ||
| expect(result.first[1][:db].size).to eq 3 |
There was a problem hiding this comment.
Such a deep nesting may be a sign that it'd be better to return structs/objects instead of hashes. WDYT?
Earendil95
force-pushed
the
extract-collector
branch
2 times, most recently
from
90d1459
to
97fedc2
Compare
Earendil95
changed the title
There was a problem hiding this comment.
Finally, I found time to read through it carefully 🙂
Awesome work 👏
I've created a new branch — v1-dev — and suggest merging it right away into it. The core functionality has been implemented; now we need to do some cleanup.
Here is the list of thing I'd like to consider for the final v1 release:
- Naming.
The word "queries" is no longer reflecting the nature of the N+1 ... events? operations?
I think, we should use a more generic term internally as well as in matchers/assertions.
For backward compatibility, we can keep xxx_queries aliases.
-
It would be great to make
show_table_statsfunctionality generic as well. Say,show_tags_stats; DB table is a tag, URI hostname could be a tag, too. -
Backtraces should be a part of the Base collector; it's useful to have them for all collectors. "Query" truncation, too.
-
Finally, abstractizing ActiveSupport Notifications away would be another move forward towards the bright Ruby (and not only Rails) future. We implement such idea in TestProf's EventProf; we can borrow an abstract interface from it.
Earendil95
force-pushed
the
extract-collector
branch
2 times, most recently
from
f747339
to
1fc25e5
Compare
…t custom collectors
Earendil95
force-pushed
the
extract-collector
branch
from
1fc25e5
to
f13717e
Compare
|
@palkan Thanks for the review! I've merged, as you suggested. Stay tuned, will post the other things |
Not necessary; PRs are better )) |
What is the purpose of this pull request?
From README.md:
This PR is going to add an ability to create custom Collectors so we can track N+1 "everywhere, for everyone, forever" (c).
What changes did you make? (overview)
Is there anything you'd like reviewers to focus on?
Not yet. This PR currently makes it easier to track my progress.
Checklist