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

Fix problem with undef override_hostname #34

Merged
merged 1 commit into from
Jan 21, 2015
Merged

Fix problem with undef override_hostname #34

merged 1 commit into from
Jan 21, 2015

Conversation

NoodlesNZ
Copy link
Contributor

If override_hostname is not defined it uses the default of undef, but the logic in the template doesn't work as expected. I've added a failing rspec test first, which I will fix in another commit.

@jhoblitt
Copy link
Owner

jhoblitt commented Dec 3, 2014

There's no rspec tests as part of this PR?

@NoodlesNZ
Copy link
Contributor Author

Yea, I failed at writing the rspec test. I wanted it to fail if override_hostname is present with the default params.

@jhoblitt
Copy link
Owner

Well, it's failing...

@NoodlesNZ
Copy link
Contributor Author

Ok, that seems to have got it (with the Travis CI problem again).

@jhoblitt
Copy link
Owner

#36 should work around the ruby 1.8.7 problem. Try rebasing on top of master.

@NoodlesNZ
Copy link
Contributor Author

Looks like #36 did the trick, good work

@jhoblitt
Copy link
Owner

Could you rebase?

@jhoblitt
Copy link
Owner

@NoodlesNZ ping? This PR needs to be rebased so it can be merged.

@NoodlesNZ
Copy link
Contributor Author

I've only just got back from vacation, so will look at this once I've caught up on emails/work.

@NoodlesNZ
Copy link
Contributor Author

There you go, that should do it.

jhoblitt pushed a commit that referenced this pull request Jan 21, 2015
Fix problem with undef override_hostname
@jhoblitt jhoblitt merged commit 668a7d4 into jhoblitt:master Jan 21, 2015
@jhoblitt
Copy link
Owner

Thank you again!

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.

2 participants