Skip to content

Conversation

@johalloran01
Copy link
Contributor

Find and replaced PasskeyReauthentication with Reauthentication, and test_passkey_reauthentication with test_reauthetntication. As well as renamed files.

Find and replaced PasskeyReauthentication with Reauthentication, and test_passkey_reauthentication with test_reauthetntication. As well as renamed files.
@tcannonfodder tcannonfodder self-requested a review June 13, 2023 23:42
Copy link
Contributor

@tcannonfodder tcannonfodder left a comment

Choose a reason for hiding this comment

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

It's almost there; but it looks like we've got a case of find & replace gone amok!

The only things that need to be renamed are:

The PasskeyReauthentication concern

Devise::Passkeys::Controllers::Concerns::PasskeyReauthentication => Devise::Passkeys::Controllers::Concerns::Reauthentication

The test class:

Devise::Passkeys::Controllers::Concerns::TestPasskeyReauthentication => Devise::Passkeys::Controllers::Concerns::TestReauthentication
  • The filenames for each
  • The requiring of "controllers/concerns/passkey_reauthentication" in lib/devise/passkeys/controllers.rb

The other changes here (making methods start with an uppercase, changing the symbol names, changing the name of the strategy) will need to be rolled back.

Let me know if you have any questions or need anything clarified!

Reran find and replace for PasskeyReauthentication to Reauthentication

Updated file names
Copy link
Contributor

@tcannonfodder tcannonfodder left a comment

Choose a reason for hiding this comment

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

There was one goof up I made in my feedback. I also need to run this on CircleCI to confirm it passes

* This actually needs to remain as
	`Devise::Strategies::PasskeyReauthentication` because it's the class
	that defines the devise strategy.
* Similar to how we kept the key as :passkey_reauthentication, the class
	needs to stay the same (especially because it's convention for the
	key & class to mirror each other).
@tcannonfodder tcannonfodder merged commit da9e622 into ruby-passkeys:main Jun 14, 2023
@tcannonfodder tcannonfodder changed the title Rename Modules Rename Devise::Passkeys::Controllers::Concerns::PasskeyReauthentication => Devise::Passkeys::Controllers::Concerns::Reauthentication Jun 14, 2023
@heliocola
Copy link
Contributor

@tcannonfodder , @johalloran01 : how pick are you about adding these changes to CHANGELOG.md? ☺️

@tcannonfodder
Copy link
Contributor

tcannonfodder commented Jun 14, 2023

I’m not sure I follow? I did add them in a later commit, post-merging: de50113 😄😅

@heliocola
Copy link
Contributor

TY! Sorry about that!
I've missed that!
I was reviewing the PR and noticed no changes to CHANGELOG.md.

@tcannonfodder
Copy link
Contributor

No worries! Just wanted to make sure I didn’t miss anything 😄

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.

3 participants