Skip to content
This repository was archived by the owner on Jun 28, 2023. It is now read-only.

feat(Connection): send feature flag on connect#2

Merged
GuillaumeSalles merged 2 commits into
masterfrom
feat/MG-20360-send-feature-flag-on-connect
Sep 27, 2017
Merged

feat(Connection): send feature flag on connect#2
GuillaumeSalles merged 2 commits into
masterfrom
feat/MG-20360-send-feature-flag-on-connect

Conversation

@GuillaumeSalles
Copy link
Copy Markdown
Contributor

@GuillaumeSalles GuillaumeSalles commented Sep 26, 2017

Send feature flag header on connect to keep them inside the falcor session.

Will be used in this PR : https://github.com/Mixgenius/LANDR.Falcor/pull/233

Did I miss something?

@ardeois
Copy link
Copy Markdown
Contributor

ardeois commented Sep 26, 2017

@GuillaumeSalles Falcor-HTTP-WS-DataSource is a public project that ideally should be agnostic from implementation.
I'm not sure adding feature flags here make sense. We should maybe add a hook mechanism or a way to enhance the authorization payload?

@GuillaumeSalles
Copy link
Copy Markdown
Contributor Author

screen shot 2017-09-26 at 4 43 12 pm

@GuillaumeSalles
Copy link
Copy Markdown
Contributor Author

Is it really used?

@ardeois
Copy link
Copy Markdown
Contributor

ardeois commented Sep 26, 2017

No it's not used because we use a private fork I wanted to remove (see closed PR #1)

I am ok if we are aware this project will/can not be used by external community anymore

Copy link
Copy Markdown

@ludovicthomas ludovicthomas left a comment

Choose a reason for hiding this comment

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

We will improve it later with a hook if there is a need for others to use it. The authorization with bearerToken is also not conventional if somebody wants to use something else.

In all cases, it's not a breaking change, as if there is no header, it will just return null and do nothing.

@GuillaumeSalles
Copy link
Copy Markdown
Contributor Author

I just bumped the version. I suppose only @synhaptein can push the new version to npm?

@GuillaumeSalles GuillaumeSalles merged commit 012e767 into master Sep 27, 2017
@GuillaumeSalles
Copy link
Copy Markdown
Contributor Author

Published

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.

4 participants