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

Deprecate 2 lookup methods #446

Merged
merged 1 commit into from
Apr 30, 2018
Merged

Deprecate 2 lookup methods #446

merged 1 commit into from
Apr 30, 2018

Conversation

joshcanhelp
Copy link
Contributor

  • WP_Auth0_Options::get_enabled_connections()
  • WP_Auth0::get_plugin_dir_url()

@joshcanhelp joshcanhelp added this to the v3-Next milestone Apr 23, 2018
Copy link
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

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

are you running the beautifier on a separate beautification PR or are you running it for each PR? Asking since one docblock has a newline and the other doesn't. BTW, this is not an issue to me :)

@@ -19,8 +19,13 @@ public function is_wp_registration_enabled() {
return get_site_option( 'users_can_register', 0 ) == 1;
}

// TODO: Deprecate, not used
/**
* @deprecated 3.6.0 - Not used
Copy link
Contributor

Choose a reason for hiding this comment

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

explain why?

@joshcanhelp
Copy link
Contributor Author

@lbalmaceda - beautification is a bit of a WIP but I will be running the whitespace one once all the PRs are in place.

- WP_Auth0_Options::get_enabled_connections()
- WP_Auth0::get_plugin_dir_url()
public function get_enabled_connections() {
// phpcs:ignore
trigger_error( sprintf( __( 'Method %s is deprecated.', 'wp-auth0' ), __METHOD__ ), E_USER_DEPRECATED );
return array( 'facebook', 'twitter', 'google-oauth2' );
Copy link
Contributor

Choose a reason for hiding this comment

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

is weird that this was (and is) hardcoding an array of enabled connections. I've read the deprecation message. However, are you currently calling this method somewhere in your plugin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't remove it so I'm not entirely sure when or where this was used. I checked through all the functions here and did not see it used anywhere in the code. I think this was part of the initial setup at some point?

This is the case with a number of this deprecations. I didn't remove them and didn't see them used anywhere. If we're not using the method internally, I don't want to maintain it 👍

Copy link
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

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

LG

@joshcanhelp joshcanhelp merged commit ad2705f into dev Apr 30, 2018
@joshcanhelp joshcanhelp deleted the deprecate-lookup-methods branch April 30, 2018 21:45
@joshcanhelp joshcanhelp mentioned this pull request Jun 5, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants