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

Cleanup in pre-defined AuthProviders. #12

Open
ghost opened this issue Dec 26, 2018 · 0 comments
Open

Cleanup in pre-defined AuthProviders. #12

ghost opened this issue Dec 26, 2018 · 0 comments

Comments

@ghost
Copy link

ghost commented Dec 26, 2018

  1. All pre-defined AuthProviders contain unused code in form of this computed property:
	private var appAccessToken: String {
		return clientID + "%7C" + clientSecret
	}

This property is never accessed by the other code.

  1. The comment style is quite inconsistent. There are /// comments and /** **/ comments.
    Notably, there are comments that seem to have been straightly copied from the Turnstile repository, e.g.
	/**
	Create a Facebook object. Uses the Client ID and Client Secret from the
	Facebook Developers Console.
	*/

In this case, the leading indentation got misaligned while copying. The original seems to be here:
https://github.com/stormpath/Turnstile/blob/master/Sources/TurnstileWeb/LoginProviders/Facebook.swift

  1. There are indicators that suggest that the various AuthProviders seem to have been copy pasted between each other. For example, there is a fb variable in non-Facebook auth providers.

  2. There is lots of commented-out code, empty lines and in case of SalesForce personal user information containing names, user IDs and coarse location information. While this is not a strictly technical problem, it does not provide the impression that this code is ready for production, especially as this code is the first thing one has to look at when integrating a custom AuthProvider.

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

No branches or pull requests

0 participants