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

Refactor requires #57

Merged
merged 5 commits into from
Nov 2, 2015
Merged

Refactor requires #57

merged 5 commits into from
Nov 2, 2015

Conversation

dentarg
Copy link
Collaborator

@dentarg dentarg commented Oct 29, 2015

With the refactoring of spec_helper, a bug like this can now be caught by

bundle exec rspec spec/lib/twingly/url/utilities_spec.rb

but I did not succeed in catching it in

bundle exec rake

We can continue to work on that from this.

Close #56.

With the refactoring of spec_helper, a bug like this can now be catched
by "bundle exec rspec spec/lib/twingly/url/utilities_spec.rb", but I did
not succeed in catching in "bundle exec rake". We can continue to work
on that from this.

Close #56.
@roback
Copy link
Member

roback commented Oct 30, 2015

I get the same error when running just the NullURL specs. (it must have access to the URL class here: https://github.com/twingly/twingly-url/blob/master/lib/twingly/url/null_url.rb#L8)

This is not a problem in production since you never should require null_url by yourself.

$ bundle exec rspec spec/lib/twingly/url/null_url_spec.rb

Failures:

  1) Twingly::URL::NullURL#trd
     Failure/Error: subject { url.trd }
     NoMethodError:
       undefined method `trd'
     # ./lib/twingly/url/null_url.rb:8:in `method_missing'
     # ./spec/lib/twingly/url/null_url_spec.rb:24:in `block (3 levels) in <top (required)>'
     # ./spec/lib/twingly/url/null_url_spec.rb:25:in `block (3 levels) in <top (required)>'
...

@walro
Copy link
Contributor

walro commented Oct 30, 2015

I get the same error when running just the NullURL specs. (it must have access to the URL class here: https://github.com/twingly/twingly-url/blob/master/lib/twingly/url/null_url.rb#L8)

This is not a problem in production since you never should require null_url by yourself.

I guess we should fix it anyway

@walro walro changed the title [WIP] Refcator requires [WIP] Refactor requires Oct 30, 2015
@walro
Copy link
Contributor

walro commented Oct 30, 2015

I guess we should fix it anyway

Done in 4172a38

@roback
Copy link
Member

roback commented Oct 30, 2015

Done in 4172a38

👍

@walro walro changed the title [WIP] Refactor requires Refactor requires Oct 30, 2015
We seem to use “require_relative” in /lib and “require” in /spec
@walro
Copy link
Contributor

walro commented Oct 30, 2015

I think this looks good now

To catch require bugs like #56.
Travis CI install all gems into vendor/, so we tried to run all sort of
specs...
@roback
Copy link
Member

roback commented Nov 2, 2015

Tested locally, LGTM :)

walro added a commit that referenced this pull request Nov 2, 2015
@walro walro merged commit 4bf6684 into master Nov 2, 2015
@walro walro deleted the refactor-requires branch November 2, 2015 09:43
@walro
Copy link
Contributor

walro commented Nov 2, 2015

Will wait with new version release until we have gotten our other ongoing PRs in

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

Successfully merging this pull request may close these issues.

None yet

3 participants