Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Remove Authentication component. #108

Merged
merged 3 commits into from
Dec 18, 2018
Merged

Remove Authentication component. #108

merged 3 commits into from
Dec 18, 2018

Conversation

T4rk1n
Copy link
Contributor

@T4rk1n T4rk1n commented Dec 13, 2018

  • Removed the Authentication component, all it did was reading the configs.
  • Moved the readConfig dispatch to AppContainer.

Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet left a comment

Choose a reason for hiding this comment

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

This looks good to me. As this neither adds a feature or fixes an issue I'm fine with not publishing this change alone. As per https://keepachangelog.com/en/1.0.0/, I think we should at least document in the changelog. Something like this:

## [Unreleased]
### Maintenance
- Remove unused Authentication component [#108](https://github.com/plotly/dash-renderer/pull/108)

@T4rk1n T4rk1n force-pushed the remove-authentication branch from 01e7fc8 to 2071021 Compare December 17, 2018 22:57
@T4rk1n
Copy link
Contributor Author

T4rk1n commented Dec 17, 2018

I looked back at the Authentication component and found a _dash-login request. There's a _dash-login route in dash-auth but it's handled by it's oauth.js bundle.

@chriddyp Can you confirm that removing this api will not break anything ?

@chriddyp
Copy link
Member

Can you confirm that removing this api will not break anything ?

It will not break anything

Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet left a comment

Choose a reason for hiding this comment

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

💃

@T4rk1n T4rk1n merged commit 7812cba into master Dec 18, 2018
@T4rk1n T4rk1n deleted the remove-authentication branch December 18, 2018 14:22
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.

3 participants