Codebase refactoring

[fuadsaud]

Recently I got interested in this project and felt interested in help with maintenance, but one of the things that bugged me the most when I cloned the repo and took a look at the codebase is that it felt messy :( So I decided to do some very basic brush-up at first, as a way to get used to the codebase while trying to improve it a little. This is the result of it. Beware that these commits break ruby 1.8 compatibility (I'm not sure how much this matters :); the build is green on my end, I did the whole refactoring trusting the current test coverage.

What I did was mainly extract classes to their own files, extract some methods, remove whitespace, and stick to some more obvious conventions. It seems like the codebase is kind of old and the basic structure has never changed. I did try to not make many big changes here, but I have another branch with more specific style fixes (guided by the ruby style guide - I'll submit another PR if this gets accepted).

Anyway, if you guys disagree with this, please share your thoughts.

[mislav]

Thanks for your interest in the project. However, 1.8 is included in the CI suite, and you must not break CI. Therefore you must keep 1.8 compatibility and not use require_relative and the like.

I might cherry-pick some style changes that I agree with, however I'm not sold on extracting classes to their own file, especially because it breaks hub standalone since its build process isn't programmed to handle nested requires.

Also, I'm not a fan of significant refactorings just for the sake of refactoring and "improving" style, since that causes mantenance nightmares down the line for me as I need to resolve extra conflicts while applying current PRs and cherry-picking between master and "1.10" branch.

[fuadsaud]

@mislav

• about 1.8: is there any good reason it's still supported?
• about build_process: I understand that it breaks the process, but is that an excuse to keep those nested classes declarations in the same file? I think it makes readability worse :\ Maybe we could improve the build process?
• Yes! Keeping branches is a nightmare, and big changes make the process a lot worse. My goal her however is to bring up the discussion about current practices being used, not necessarily merging everything at once. I just didn't know a better option to do this.

[mislav]

1. Some systems still ship with 1.8 out of the box. Also, you don't actually need require_relative in your code. A simple require "hub/foo" will suffice.
2. Sure, I'll accept a change that improves the build process in such a way that it handles nested requires and adds tests that the resulting script is valid. It's a failure of our test suite that it still passes after you broke it.
3. So you know that small, focused changes isolated in separate commits are likelier to get accepted than bigger refactorings and moving code around.

[fuadsaud]

1. Yes, I know it's not needed. I just normally prefer it over regular require;
2. Nice. I didn't look at all the build script; I'll see if can do something soon;
3. I know. I understand this pr brings in big and bold changes, but, like a said, the point of this request is mainly to suggest move things around. I tried to have a commit corresponding to each file I touched, so it could be more granularly leveraged.

Anyway, I'm not expecting you to just click the merge button and integrate all the changes at once. Like I said, my point is to raise the discussion about the current state of the codebase. I think there is room for improvement in the points I touched (and more); you may disagree with my opinion.

[mislav]

Bit of bad news for this PR. Recently we've decided that the next major release of hub will be written in Go instead of Ruby: #475

This work is being done in the "gh" branch. As a consequence of that, we're not going to make major improvements to the Ruby codebase. It doesn't make sense, since we're not going to mainain it in the long run except bug fixes for the 1.11.x branch of releases.