Skip to content
This repository has been archived by the owner on Nov 8, 2018. It is now read-only.

Tidy up code for remembering the IDP #45

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

kentsanggds
Copy link
Contributor

What

Refactored the gateway code dealing with remembering the IDP to make it clearer

How to review

Look at the code in app/main/views/auth.py

Execute run-tests to ensure that code changes do not break the website

@kentsanggds kentsanggds requested a review from andyhd March 1, 2017 11:53
dept = [profile['name']
for profile in idp_profiles if idp_name == profile['idp_name']]
if dept:
return ' or '.join(dept)
Copy link
Contributor

Choose a reason for hiding this comment

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

Arbitrary indentation level on line 101. But rather than using a list comprehension, consider using a for loop instead - list comprehensions are less readable. Also, since the callers of this function do not handle a None return value, the check on line 102 is redundant. Eg:

dept_names = []

for profile in idp_profiles:

    if idp_name == profile['idp_name']:
        dept_names.append(profile['name'])

return ' or '.join(dept_names)

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.

2 participants