-
Notifications
You must be signed in to change notification settings - Fork 21.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Remove method missing use in respond_to
- Loading branch information
Showing
1 changed file
with
23 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congrats on the first commit :-)
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
He gets straight to business!
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weee
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, I really like the inline docco sample of the generated method that will be created by #generate_method_for_mime
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn’t you make method_missing private?
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@drnic
I hope from this point, every metaprogramming trick will have such comment. I personally find it very hard to read metaprogramming code otherwise.
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for human-readable meta comment. It’d certainly be nice to have this used throughout the code base. Great job… and nice first commit.
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expectations are high for wycats’ second commit to Rails. We’ll be watching for it with baited breath.
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m waiting for the failing test which prevents method_missing from ever being used :)
The inline sample doc is really nice too, sets great precedent. I wonder if there could be a failing test to enforce that too – any call to def inside a class eval must have a trailing comment…
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for inline meta comments
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those comments are a great idea. Inspired by this commit I’m adding them in more places through docrails.
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an awesome blossom.
6dc1288
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OMG lulz CONGRATS!!!
In all seriousness, the commented code is cool, more plz