piotrmurach / tty-progressbar

Display a single or multiple progress bars in the terminal.
https://ttytoolkit.org
MIT License
424 stars 24 forks source link

Naming and design #18

Closed ioquatix closed 7 years ago

ioquatix commented 7 years ago

Hi, it's me again :D

I'd just like to kindly suggest that having a file named lib/tty-progressbar.rb is an anti-pattern, and also, a class called ProgressBar should be in a file called progress_bar.rb not progressbar.rb. Additionally, rather than overloading progress bar to be a spinner, you might want to have an entirely separate class. I think it's a nicer design.

Just some general feedback. Really useful gem.

ioquatix commented 7 years ago

Hmm, I see that spinner is already separate, for some reason I thought it was an option on progress bar. My bad, ignore that feedback.

piotrmurach commented 7 years ago

Ohh noooo you again .... :smile:

I would still keep the lib/tty-progressbar.rb for loading compatibility but will change it to only require tty/progressbar and use relative paths :wink: . I agree with the convention, my oversight on the class vs file naming.

Cheers for the feedback!