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

Improve DEPR process for toggles #283

Open
2 tasks
robrap opened this issue Jun 29, 2023 · 3 comments
Open
2 tasks

Improve DEPR process for toggles #283

robrap opened this issue Jun 29, 2023 · 3 comments

Comments

@robrap
Copy link
Contributor

robrap commented Jun 29, 2023

The following are a number of ideas for improving deprecation for toggles.

  1. These could be split out into separate tickets as needed.
  2. These were originally sourced from https://openedx.atlassian.net/wiki/spaces/AC/pages/2554331530/Toggles+and+Settings+Documentation.

Questions:

  • How can we ensure that newly added temporary toggles are handled by maintainers, and don't become random duture DEPR work?
    • Can we have an SLA for maintainers for removing temporary toggles? Especially to apply to new ones.
    • What's the best way to move forward on this?

Proposals:

  • Add annotation for toggle_target_value (get name reviewed) to document whether to keep the True or False case once removed.
    • This would be 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?
    • We could just document that this detail should be added to the description for now.
  • Ensure (lint) a properly formatted date for toggle_target_removal_date annotation.
  • Here are examples with toggle_target_removal_date of None, even though it is required.
    • Note: This whole section is meant to be under the lint toggle_target_removal_date bullet, but indentation isn't working correctly.
      • Regarding linting, what if people want to use a named release instead? Should they just use its date?
      • 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 since updated with proposal for how to choose a date.
      • Linting failure help text could point to these docs.

Additional Notes:

Moved to separate tickets:

@robrap
Copy link
Contributor Author

robrap commented Oct 17, 2023

Also relates to #315 where these annotations may all get used to find temporary toggles for removal.

@robrap
Copy link
Contributor Author

robrap commented Nov 2, 2023

+1 for toggle_depr_ticket annotation from today's meeting to review. People have used the existing annotation for a variety of tickets, including tickets for context, which may be useful but is different from quickly knowing if a DEPR ticket exists or does not exist.

@dianakhuang
Copy link
Contributor

dianakhuang commented Nov 16, 2023

  • Add annotation for toggle_depr_ticket to make it simpler to know if a toggle has an associated DEPR ticket.
    • Should this field be required for temporary toggles?
    • Is it better to create the DEPR ticket when creating the toggle?
    • Is it ok to use private tickets for short-lived toggles, or do we always want DEPRs for every newly created temp toggle?
    • Could there be an abbreviated process for new temporary toggles where they don't need to be announced?

Q: are temporary toggles added by anyone other than 2U?
A: yes, but usually these are added at the behest of 2U for rollout/safety purposes

Proposal: add the field and make DEPR tickets, but as we move forward, reconsider whether or not we want to make it required.

Recommend calling this field toggle_removal_ticket instead.

UPDATE: The new ticket for this work is #324

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Proposed
Development

No branches or pull requests

2 participants