OCD #412
OCD #412
Conversation
|
I like these. Especially 96c8833, I needed that to run the tests. |
|
Just to provide some historical context as this lingers open, this wasn't just for giggles, I did most of this in the context of several other things I worked on (plus one other that I'm not even bothering to submit a PR for yet, because it's a large changeset and maintenance of this library is flagging at this point in time, four pull requests of mine open for two months with no feedback from maintainers...). |
|
Can you rebase against master and force push? |
|
...aaand rebased this one too. |
|
Ok, new stuff in master that is included in your branch, rebase && push again, plz 😈 I do have a few questions about other parts of the code, will discuss then. |
|
Rebased again. |
|
Um, so, forgot to discuss 'then' sorry. Looks like we can't automatically merge again. (But wait for #438 before rebasing) |
|
So sorry, rebase again? |
This was doing nothing dynamic, and the eval is wastefully repeated if there are multiple calls to `acts_as_taggable_on` with a context. The helper for these methods is already a regular instance method not used anywhere else, so the `tag_types.empty?` guard wasn't exactly a huge win in saving namespace pollution.
Know an Ruby module. This stuff confuses and misguides newcomers. http://yehudakatz.com/2009/11/12/better-ruby-idioms/
|
Alright, rebased again. dd958e4 in particular is likely to be annoying to keep rebasing, so I may lose patience, to be honest. |
|
@ches, I totally get that... reviewing |
|
Looks good. @seuros Any objections? |
|
Thanks for the persistence 😄 I hope the changes don't cause too much grief for people rebasing other PRs, but git seemed to figure out indentation change from nuking |
|
Yeah, you too. At some point it's better to merge something now and clean it up later than wait for perfection and cause everyone grief :) |
[Misc] Code cleanup, refactoring
You're going to hate me 😓 Compulsion got the better of me. Just threw the branch out there, cherry-picking certainly an option.