Skip to content

has_secure_password should allow optional password confirmation #2837

Closed
wants to merge 4 commits into from

3 participants

@edgarjs
edgarjs commented Sep 3, 2011

I don't think password_confirmation should be a must on all sign ups, if you implement the reset password functionality you don't need to confirm the user's password.

That''s better for a usability point of view since you save time to the user on the sign up, don't make them type more.

@smartinez87

Can you please update the docs as well and include it to your pull request?
More specifically the lines below this one: https://github.com/edgarjs/rails/blob/ede56f1df0ba0039411697744fe0e5764b96ebbb/activemodel/lib/active_model/secure_password.rb#L9

@edgarjs
edgarjs commented Sep 3, 2011

Committed, should I create another pull request? or can I update this one?

@edgarjs
edgarjs commented Sep 3, 2011

Figured out. Updated.

@smartinez87

Best would be to squash the commits into only 1

@edgarjs
edgarjs commented Sep 4, 2011

Sorry, I don't have experience squashing commits. I did it but it merged with the origin, not sure if it's ok.

@dasch
dasch commented Sep 4, 2011

@edgarjs do it like this: run git rebase -i master. Your editor will open with the list of commits - replace pick with squash before all but the first commit, save, and quit. That should take care of it.

@edgarjs
edgarjs commented Sep 5, 2011

hey @dasch, yes I did that, but then it said my branch have diverged, so I needed to do a pull before the push, which caused another merge commit. That's what I'm not sure about how to avoid it.

@dasch
dasch commented Sep 5, 2011
@edgarjs
edgarjs commented Sep 5, 2011

Now I can't do the squash again, it says:

noop

# Rebase 8d49701..8d49701 onto 8d49701
#
# Commands:
# p, pick = use commit
# r, reword = use commit, but edit the commit message
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
# f, fixup = like "squash", but discard this commit's log message
# x, exec = run command (the rest of the line) using shell
#
# If you remove a line here THAT COMMIT WILL BE LOST.
# However, if you remove everything, the rebase will be aborted.
#

Sorry about the lack of knowledge in git.

@edgarjs
edgarjs commented Sep 5, 2011

Somehow I did it... need to review the steps and see where I pushed it, because it got to the origin but somehow before the other commits.. or maybe I did something weird. Anyway, here's the single commit: #2879

Sorry for creating another pull request, not found where to remove commits from this one.

@edgarjs edgarjs closed this Sep 5, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.