Skip to content
This repository has been archived by the owner on Apr 22, 2020. It is now read-only.

#489 make taupage config option naming consistent #494

Merged
merged 1 commit into from
Apr 10, 2018

Conversation

smirnov
Copy link
Member

@smirnov smirnov commented Apr 10, 2018

for enabling introspection, remove faulty support

@@ -14,7 +14,7 @@ script
export OPENID_PROVIDER_CONFIGURATION_URL="$config_openid_provider_configuration_url"
export REVOCATION_PROVIDER_URL="$config_revocation_provider_url"
export BUSINESS_PARTNERS="$config_tokeninfo_business_partners"
if [ "$config_tokeninfo_introspection" = "true" ] || [ "$config_tokeninfo_introspection" = "True" ]; then
if [ "$config_planb_local_tokeninfo_introspection" = "true" ] || [ "$config_planb_local_tokeninfo_introspection" = "True" ]; then

Choose a reason for hiding this comment

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

with the config be named planb_local_tokeninfo_introspection? For consistency shouldnt it be local_planb_tokeninfo_introspection

Copy link
Member

Choose a reason for hiding this comment

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

iMO it should be local_planb_tokeninfo: true and local_planb_tokeninfo_introspection: true, yes

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed.

@smirnov smirnov force-pushed the feature/489-rename-introspection-parameter branch from 9a6adf7 to 4709979 Compare April 10, 2018 09:30
@smirnov
Copy link
Member Author

smirnov commented Apr 10, 2018

👍

1 similar comment
@jbspeakr
Copy link
Member

👍

@aermakov-zalando aermakov-zalando merged commit 978d7b9 into master Apr 10, 2018
@aermakov-zalando aermakov-zalando deleted the feature/489-rename-introspection-parameter branch April 10, 2018 10:01
@hjacobs
Copy link
Contributor

hjacobs commented Apr 10, 2018

Is this option documented on http://stups.readthedocs.io ?

@smirnov
Copy link
Member Author

smirnov commented Apr 10, 2018

No and neither is local_planb_tokeninfo. This can and probably should be tackled together.

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.

5 participants