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

Remove tests for undocumented private methods #5962

Merged
merged 1 commit into from May 2, 2012
Merged

Conversation

bogdan
Copy link
Contributor

@bogdan bogdan commented Apr 24, 2012

hash_for_* methods are private and not designed to be called by end user.

Their API can be changed without backward compatibility, that is why they should not be tested.

@josevalim
Copy link
Contributor

I am not sure if these are private API. They are protected because we don't want to be able to call them externally and they don't have docs because they are automatically generated, so unsure if this is the way to go.

@jeremy
Copy link
Member

jeremy commented Apr 24, 2012

What's its history? Why is it protected?

@josevalim
Copy link
Contributor

I think those and all url helpers were always protected. c/d?

@bogdan
Copy link
Contributor Author

bogdan commented Apr 25, 2012

It's true. Seems like these methods were always protected. And it's possible to call it from a controller or view.

In 3rd commit adds test for this method, but that was not actually the place where it appeared.
1st and 2nd commit looks like a followups.
Before these commits it seems like this method had a different API - see 4th. It's hard to track.

The code itself looks almost in the same way as in 2006.

I personally don't feel like this is something people relay on in apps.
Can't imagine the use case when this method can be called from controller or a view.

1st
commit 8c4b599
Author: Rick Olson technoweenie@gmail.com
Date: Sun Aug 13 18:31:58 2006 +0000

Fix assert_redirected_to issue with named routes for module controllers.  [Rick Olson]

2nd
commit e494b0a
Author: Nicholas Seckar nseckar@gmail.com
Date: Fri Jun 30 02:36:17 2006 +0000

Add route_name_path method to generate only the path for a named routes. For example, map.person will add person_path.

3rd
commit b20c575
Author: Jamis Buck jamis@37signals.com
Date: Thu Jun 1 15:42:08 2006 +0000

New routes implementation. Simpler, faster, easier to understand. The published API for config/routes.rb is unchanged, but nearly everything else is different, so e

4th
commit 8e56f5e
Author: David Heinemeier Hansson david@loudthinking.com
Date: Fri Jun 24 16:40:01 2005 +0000

Improved performance of Routes generation by a factor of 5 #1434 [Nicholas Seckar] Added named routes (NEEDS BETTER DESCRIPTION) #1434 [Nicholas Seckar]

@jeremy
Copy link
Member

jeremy commented Apr 25, 2012

+1 to remove :)

@bogdan
Copy link
Contributor Author

bogdan commented Apr 27, 2012

I am currently preparing a patch that makes hash_for_* methods unnecessary

So, do you want me to deprecate them or just remove?
I would personally vote for last option.

josevalim added a commit that referenced this pull request May 2, 2012
Remove tests for undocumented private methods
@josevalim josevalim merged commit 3b6a353 into rails:master May 2, 2012
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