Page Comparison

...

• Need to add deprecation ticket to a separate annotation for convenient filtering out during future reviews

• DEPR

  (HIGH)

  Add annotation for toggle_target_value (get name reviewed) to document whether to keep the True or False case once removed.

  • Required for temporary toggles. Is it useful to have this in the code, or is some of this DEPR metadata better kept in the DEPR ticket alone?

• (HIGH) Add annotation for toggle_depr_ticket to make it simpler to know if a toggle has an associated DEPR ticket.

  • Required for temporary toggles? Would it be better to create the DEPR ticket when creating the toggle?

Can we have an SLA for maintainers for removing temporary toggles? Especially to apply to new ones

DEPR improvement ideas moved to https://github.com/openedx/edx-toggles/issues/283.

• (HIGH) Ensure undefined annotations are not allowed.

  • For example, if toggle_warnings were used instead of toggle_warning, linting would complain.

  • Without this, the annotation would be visible in code, but it wouldn’t make it to readthedocs and you wouldn’t know it without checking.

  • Removing the unnecessary ability to add comments in-between annotations would simplify the regex update required for this.

  • Note: I think this may be causing us not to catch other linting issues with bad indents.

• (MEDIUM) Possible bug where required fields for temporary use case might be missed when multiple use cases are listed.

  • Note: This is not high priority, because we should prioritize switch to new toggle_life_expectancy annotation, which will make this go away.

• (MEDIUM) Ensure optional annotations are not set to None.

  • Applies to specific annotations where None is not a valid value (e.g. toggle_warning).

  • Examples with toggle_target_removal_date of None, even though it is required.

    Many of the examples also added unnecessary text to toggle_warnings that toggle_target_removal_date was not set.

  • Possibly authors documented during docathon, and were afraid to set a date.

  How-to was updated with proposal for how to choose a date. Linting failure help text could point to these

  See example of where None is copied often and leads to a lot of extra code and useless docs.

  [RG]* (HIGH) Ensure a properly formatted date for toggle_target_removal_date annotation.

  • docs.

• Check toggle_implementation annotation matches actual implementation class used when possible.

  • Note: examples of manually corrected errors.

• No linting exists to ensure settings are annotated.

  • Needs brainstorming if we think this would be important.

• No linting exists to ensure ConfigModels with booleans are annotated.

...

Update Toggle and Setting Annotation Definition

• [RG]* (HIGH) Implement ADR 0005-toggle-life-expectancy.rst.

  See ADR Consequences for details.

Toggle Enhancements

• (HIGH) Enhance BaseToggle with additional methods.

  • Add is_disabled (not is_enabled), is_on (is_enabled) and is_off (not is_enabled)

    • Question: Would is_true() and is_false() be better than is_on/is_off?

      • [Robert Raposa] I like is_true/is_false better. Someone else should second my opinion and we can update this proposal.

  • Makes a variety of conditions more readable when ENABLE_ or DISABLE_ are in the toggle name.

  • For example:

    • not DISABLE_SOME_FEATURE.is_enabled() # BEFORE; brain twister

    • DISABLE_SOME_FEATURE.is_off() # AFTER

      • Question: Is this simpler to understand?:

        • DISABLE_SOME_FEATURE.is_false() # AFTER

  • This problem was discussed on Slack.

• Replace or improve ExperimentWaffleFlag

  • See Learning MFE toggle feedback