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

Require all in steam-condenser #10

Closed
wants to merge 2 commits into from

Conversation

jpalumickas
Copy link
Contributor

This must be included, because now showing uninitialized constant SteamCondenser::Servers when requiring only steam-condenser.

This must be included, because now showing `uninitialized constant SteamCondenser::Servers` when requiring only `steam-condenser`.
@koraktor
Copy link
Owner

I guess there needs to be some work done before loading the library is really straight-forward. To be honest I don't think require everything is the way to go.

But maybe we can use this to discuss a bit on this issue?

I'm still not really sure what require 'steam-condenser' should give you without loading too much. But for sure, require 'steam-condenser/community' and require 'steam-condenser/servers' should give you everything needed for those areas.

@jpalumickas
Copy link
Contributor Author

I mean if I set require 'steam-condenser' I get an error uninitialized constant SteamCondenser::Servers. So for default it should be all required and if someone don't need to load all gem, he could do this:

gem 'steam-condenser', require: false and then require 'steam-condenser/servers'

@koraktor
Copy link
Owner

This would be a good alternative, too. In this case we should remove lib/steam-condenser/all.rb completely, because lib/steam-condenser.rb is for everything.

@jpalumickas
Copy link
Contributor Author

So you tell that we can add

require 'steam-condenser/community/all'
require 'steam-condenser/servers/all'

to lib/steam-condenser.rb and remove lib/steam-condenser/all.rb

@koraktor
Copy link
Owner

Correct.

@jpalumickas
Copy link
Contributor Author

Ye, that would be good, I can make a commit if you want.

@koraktor
Copy link
Owner

Sure, thanks. Just amend the commit and push to the same branch. That way it will stay connected with this PR.

@jpalumickas
Copy link
Contributor Author

It's not good to amend on pushed commit to remote, because amend actually rewrite the history graph, so I made another commit with fixes that we discussed. Please check and tell me what do you think about it.

@koraktor
Copy link
Owner

Thanks. I'll have a look.

PS: It's totally fine to push amended commits or rebased branches if it's announced or similar. GitHub will even update PRs so anyone can see this.

@jpalumickas
Copy link
Contributor Author

Yup, I know that, but I use merge instead of rebase to save the history. Both are good 😄 So, when you will check this code, please write your opinion.

* Ignore coverage folder
* Remove all.rb from steam-condenser
@koraktor
Copy link
Owner

Thanks. I just merged your changes.

@koraktor koraktor closed this Sep 13, 2013
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