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

Shorten all authentication helpers #1719

Merged
merged 1 commit into from
Jun 11, 2012
Merged

Shorten all authentication helpers #1719

merged 1 commit into from
Jun 11, 2012

Conversation

radar
Copy link
Contributor

@radar radar commented May 31, 2012

Previously, authentication helpers were long and tedious to type out: new_refinery_user_session_path_the_quest_for_a_longer_method_name_part_two. It is unnecessary for them to be this long. login_path will do just fine, because the engine is now isolated. There's three new helpers: login_*, logout_* and signup_*.

I don't know how you deal with deprecation of the old ones, so I've just gone ahead and changed them now.

@parndt
Copy link
Member

parndt commented May 31, 2012

@ugisozols how would you like to write deprecations for the old paths?

@travisbot
Copy link

This pull request passes (merged f928ab1 into 80416c8).

@ugisozols
Copy link
Member

No idea but we'll figure out something.

@parndt
Copy link
Member

parndt commented Jun 2, 2012

@ugisozols were you saying that you worked out deprecations?

@ugisozols
Copy link
Member

Haven't figured it out ...

@ugisozols
Copy link
Member

Can we merge this in master without deprecations and figure out 2.0.x deprecations later?

@parndt
Copy link
Member

parndt commented Jun 4, 2012

No it would be a nasty surprise for 2.1 people. I'd prefer the patch
were complete before merging...

@parndt
Copy link
Member

parndt commented Jun 7, 2012

This can't merge anymore; I guess we changed something!

@parndt parndt merged commit f928ab1 into refinery:master Jun 11, 2012
@parndt
Copy link
Member

parndt commented Jun 11, 2012

Merged - maybe we can deprecate but we can for sure blog + tweet + changelog it

ugisozols added a commit that referenced this pull request Jun 11, 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.

4 participants