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 regression #113

Merged
merged 1 commit into from
Sep 16, 2015
Merged

Fix regression #113

merged 1 commit into from
Sep 16, 2015

Conversation

kirillplatonov
Copy link
Contributor

PR #45 fixed issues #27 and #71. But related code was removed from
master at some point. Bring it back.

PR #45 fixed issues #27 and #71. But related code was removed from
master at some point. Bring it back.
@tagliala
Copy link
Collaborator

Removed here: 9aa339d

@enriclluelles ?

@enriclluelles
Copy link
Owner

trying to recall why, but it's been so long since I used the code that I don't remember :P

@kirillplatonov
Copy link
Contributor Author

So, @tagliala @enriclluelles is it good enough to be merged?

tagliala added a commit that referenced this pull request Sep 16, 2015
@tagliala tagliala merged commit 8a9c539 into enriclluelles:master Sep 16, 2015
@tagliala
Copy link
Collaborator

Merged like there ain't no tomorrow, thanks!

@kirillplatonov
Copy link
Contributor Author

@tagliala maybe it's time for new version? Patch at least?

@tagliala
Copy link
Collaborator

I think I don't have the permission to push on rubygems

@enriclluelles
Copy link
Owner

Hey @tagliala, I'll add you later today!

@@ -24,4 +24,11 @@ def set_locale_from_url
I18n.locale = current_locale if tmp_locale
end
end

class TestCase
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirillplatonov

Is this code still needed?

Tests pass without this block

@tagliala
Copy link
Collaborator

@kirillplatonov could you please give a try to

gem 'route_translator', github: 'enriclluelles/route_translator', branch: 'feature/remove-test-case'

All tests are still passing, the coverage does not include that code and specs of other applications of mine based on route_translator still pass.

@tagliala
Copy link
Collaborator

I'm going to release a new version without the TestCase class, even without feedback

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.

4 participants