search

TylerBrock / mongo-hacker

MongoDB Shell Enhancements for Hackers
tylerbrock.github.io/mongo-hacker
MIT License
1.79k stars 235 forks source link

Add "delta counts" to the "count document" commands #148

Closed pvdb closed 8 years ago

pvdb commented 8 years ago

Hey @TylerBrock,

This is an extension that I've been sitting on for a while now, but people I pair with find it very useful, so I'm submitting it for your consideration... in a nutshell, the count documents command now includes - for each collection - the "delta count" since it was last invoked.

A picture speaks a thousand words, or so they say :smile:

screen shot 2016-02-10 at 12 47 39

I find the feature most useful to "visualize" the effect of performing application actions on the collections in the database, e.g. placing an order (in the application) might create a new customer, a new order, and several new order lines (in the respective database collections).

Running count documents before and after performing the application action gives you a nice summary of that dynamic.

Hope you like it,

@pvdb

PS - as you'll see in the code, you can set mongo_hacker_config['count_deltas'] = false to suppress/disable this new feature, in which case count documents behaves exactly as before.

TylerBrock commented 8 years ago

COOL, i had some comments, but otherwise i love it. Thanks @pvdb !

TylerBrock commented 8 years ago

Is this one ready to go? Re-based on the new code?

TylerBrock commented 8 years ago

NVM, this needs an update. I'll wait :-)

pvdb commented 8 years ago

Is this one ready to go? Re-based on the new code?

Not quite, no, I still need to do a wee bit more work to implement the "multi-column" layout/formatting... stay tuned!

pvdb commented 8 years ago

NVM, this needs an update. I'll wait :-)

That's now done in #150, ready for review and sign-off... once that's done, I'll merge it into this PR and implement the "dynamic" padding mentioned in an earlier comment! :smile:

pvdb commented 8 years ago

Hey @TylerBrock, any thoughts on #150? once it's been signed off and merged, I can pull it into this PR to implement the "dynamic" padding for the delta counts... cheers, @pvdb.

TylerBrock commented 8 years ago

Ok #150 has been merged, rebase away!

pvdb commented 8 years ago

This is now ready to go, @TylerBrock - since your last review I have fixed the layout/formatting of the delta counts (by leveraging the multi-column support from #150) and have also documented the delta count feature in the README (as it is now disabled by default, meaning people will most likely not realize it's there) ... the overall diff is now a lot cleaner as well, which is always a bonus! :smile:

pvdb commented 8 years ago

I have also uploaded a new screenshot to the PR description to visually illustrate how the "dynamic" padding for delta counts now works... it looks a lot better, IMO, so the hard work was worth it!

TylerBrock commented 8 years ago

Yes! Yes it does! On Wed, Feb 10, 2016 at 5:33 AM Peter Vandenberk notifications@github.com wrote:

I have also uploaded a new screenshot to the PR description to visually illustrate how the "dynamic" padding for delta counts now works... it looks a lot better, IMO, so the hard work was worth it!

— Reply to this email directly or view it on GitHub https://github.com/TylerBrock/mongo-hacker/pull/148#issuecomment-182376559 .

pvdb commented 8 years ago

Hey @TylerBrock, are you happy to merge this PR, or did you have any further concerns/improvements in mind? Thanks, @pvdb

TylerBrock commented 8 years ago

I wasn't sure if it was updated/ready. Although now that I think it is I'll take a look.

pvdb commented 8 years ago

I wasn't sure if it was updated/ready. Although now that I think it is I'll take a look.

Cool... yep, let's :shipit: :smile:

pvdb commented 8 years ago

Poke @TylerBrock :shipit: :smile:

TylerBrock commented 8 years ago

Hey @pvdb, sorry i haven't had a chance to look at this yet. Super busy time for me. I can probably get to it this week/weekend.

TylerBrock commented 8 years ago

In the meantime, I think my only comment was going to be on the nested ternary operators. I'm not a fan, if there is any way you can clean that up or avoid it I'd appreciate the change.

pvdb commented 8 years ago

Hey @TylerBrock,

You wrote:

I'm not a fan, if there is any way you can clean that up or avoid it I'd appreciate the change.

Fair enough... I am, a fan that is :smile:, but I've refactored the function in 7d7f622 for review... thanks!

TylerBrock commented 8 years ago

Ok, sorry for being pedantic. Thank you. I think it will be easier to maintain in the longer run like this. Thank you for the hard work!

pvdb commented 8 years ago

Thanks for merging!

Ok, sorry for being pedantic. Thank you.

No problem... all part of being an effective BDFL :wink: