Skip to content
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

Various fixes #2

Merged
merged 6 commits into from
Dec 15, 2014
Merged

Various fixes #2

merged 6 commits into from
Dec 15, 2014

Conversation

jirutka
Copy link

@jirutka jirutka commented Dec 13, 2014

Here’s the second round.

Fix bad class method definitions

Good explanation is in the Ruby Style Guide.

Move colored_text to core_ext subdir and rename to colored_string

There's a good convention to put monkey-patching of core classes into core_ext subdirectory, to be very clear what's going on.

Remove unnecessary monkey-patching of String

Monkey-patching should be used very cautiously and sparingly, especially in the case of core classes. When you monkey-patch some class (e.g. add some method to it), it doesn’t change it only in the current context (file), but globally!

Fix camelCase method names

This is strong Ruby convention.

Remove unreasonable includes to main

I’m not sure if you actually know what include does. There’s a good explanation on StackOverflow. When you include some class/module on the top-level (i.e. not inside another class/module), then you include it into the top-level object called main. This is not scoped to the current file, it’s global! This is useful only for DSLs, in the most of other cases it’s inappropriate and messy.

jxxcarlson pushed a commit that referenced this pull request Dec 15, 2014
@jxxcarlson jxxcarlson merged commit c005ce2 into jxxcarlson:master Dec 15, 2014
@jirutka jirutka deleted the patch-3 branch December 16, 2014 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants