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

Flag initialisation #948

Merged
merged 14 commits into from
Feb 6, 2020
Merged

Flag initialisation #948

merged 14 commits into from
Feb 6, 2020

Conversation

ptbrowne
Copy link
Contributor

@ptbrowne ptbrowne commented Feb 6, 2020

cozy-flags now supports initialisation either

  • from the DOM attribute (data-cozy or data-flags injected by the stack)
  • from the remote endpoint /settings/flags

A CozyClient plugin can be attached to a CozyClient instance for
automatic initialisation and reset of flags on login/logout.

New APIs

  • async flags.initialise() : initialises flags from DOM or remote (remote only if DOM not possible)
  • client.registerPlugin(flags.plugin)

@ptbrowne ptbrowne requested a review from drazik as a code owner February 6, 2020 11:09
@ptbrowne ptbrowne changed the title Initializations methods Flag initialisation Feb 6, 2020
@ptbrowne ptbrowne requested a review from edas February 6, 2020 12:20
packages/cozy-flags/src/flag-codemod.js Outdated Show resolved Hide resolved
* Enables a list of flags
*
* Supports passing either
* - an array containing flag names (in this case, flag values will be true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Je trouve les deux prototypes trop différents.

J'aurais compris :

  • soit un nom soit une liste de noms
  • soit un objet nom/valeur soit un tableau d'objet nom/valeur

Là tu croises les possibilités et ça me gêne. Ca mérite probablement deux fonctions séparées

packages/cozy-flags/src/flag.js Outdated Show resolved Hide resolved
packages/cozy-flags/src/flag.js Show resolved Hide resolved

const store = new FlagStore()
export const getTemplateData = attr => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Cette fonction est toujours là pour les flags, pourquoi ne pas fixer la valeur à "flags" ? A-t-on besoin du paramétrage ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Je vois pas l'avantage de rigidifier getTemplateData alors qu'en l'état elle fonctionne et est flexible.

packages/cozy-flags/src/flag.js Outdated Show resolved Hide resolved
packages/cozy-flags/src/flag.js Show resolved Hide resolved
ptbrowne and others added 2 commits February 6, 2020 16:21
Co-Authored-By: Éric D. <15271+edas@users.noreply.github.com>
@ptbrowne ptbrowne merged commit d066952 into master Feb 6, 2020
@ptbrowne ptbrowne deleted the init-flags-dom-remote branch February 6, 2020 17:22
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

Successfully merging this pull request may close these issues.

None yet

3 participants