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

Improve code samples #15

Merged
merged 6 commits into from
Jan 22, 2017
Merged

Improve code samples #15

merged 6 commits into from
Jan 22, 2017

Conversation

stomar
Copy link
Contributor

@stomar stomar commented Sep 26, 2015

Major revision of code samples:

  • fix some bugs
  • fix style (while loops etc.)
  • increase readability by introducing some variables ...
  • avoid "include Curses" in top-level namespace
  • ...

All samples tested with Ruby 2.2.

Fix some bugs and make minor improvements.
Use iterators or "loop" instead of "for" and "while" loops,
consistently use double quotes, drop unnecessary "then" etc.
Introduce some variables and methods, simplify file slurping
and random position generation, drop some comments etc.
* win.addstr(my_str)
* # or even
* win << "\nORLY"
* win << "\nOH REALLY?"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not that easy to understand for non-native speakers... had to look it up...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it's a good change

@drbrain
Copy link
Member

drbrain commented Apr 1, 2016

I like these improvements.

You could also swap the ARGV code for ARGF and simplify, but I'm unsure if that would be helpful for new rubyists

@stomar
Copy link
Contributor Author

stomar commented Apr 1, 2016

I only split the assignments in the basic example, not in the more elaborate ones. ok?

I essentially never use ARGF, so I'm not sure about it; but do feel free to change as you like :)

@stomar
Copy link
Contributor Author

stomar commented Apr 1, 2016

(Travis CI seems to be broken since a couple of builds...)

@drbrain
Copy link
Member

drbrain commented Apr 1, 2016

Travis seems to be missing a library now, I'll check it later and merge

I'll look at switching to ARGF then.

Thanks for your work!

@stomar
Copy link
Contributor Author

stomar commented Aug 17, 2016

@drbrain This PR is almost a year old, and you essentially already approved. Could you please merge?
I'd like to get this off my list.

PS. The last commit certainly is not to blame for the failing CI.

@shugo shugo merged commit 5d692b9 into ruby:master Jan 22, 2017
@shugo
Copy link
Member

shugo commented Jan 22, 2017

@stomar I've merged your pull request. Thanks for your contribution!

@stomar stomar deleted the fix-samples branch January 29, 2017 11:57
@stomar
Copy link
Contributor Author

stomar commented Jan 29, 2017

@shugo Thanks! And nice to see this is still maintained and improved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants