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

Deprecation 2.0 #466

Open
bms63 opened this issue Oct 7, 2024 · 0 comments · May be fixed by #467
Open

Deprecation 2.0 #466

bms63 opened this issue Oct 7, 2024 · 0 comments · May be fixed by #467

Comments

@bms63
Copy link
Collaborator

bms63 commented Oct 7, 2024

Discussed in pharmaverse/admiral#2329

Originally posted by bms63 February 6, 2024

Strategy pre-1.0.0

From programming strategy guide - https://pharmaverse.github.io/admiraldev/articles/programming_strategy.html#deprecation

=====

As admiral is still evolving, functions or arguments may need to be removed or replaced with more efficient options from one release to another. In such cases, the relevant function or argument must be marked as deprecated. This deprecation is done in three phases over our release cycles.

Phase 1: In the release where the identified function or argument is to be deprecated there will be a warning issued when using the function or argument using deprecate_warn().

Phase 2: In the next release an error will be thrown using deprecate_stop().

Phase 3: Finally in the 3rd release thereafter the function will be removed from the package altogether.

Information about deprecation timelines must be added to the warning/error message.

Note that the deprecation cycle time for a function or argument based on our current release schedule is 3 months.

=====

  • This was a move fast and break things type of process.

    • Pros: Got rid of code bloat, remove future maintenance needs
    • Pros: Removed duplicated functions and helped align argument names
    • Cons: Instability that users found frustrating
    • Cons: Deprecation Cycle was very tedious to maintain

Strategy at 1.0.0

This has not been codified into any guide only discussed on GitHub.

  • Functions and arguments that we created pre-1.0.0 are being carried through with our deprecation cycle (see above)

  • We are currently embracing superseded as a way to recommend functions to users that replace current functions

  • In our new paradigm we are no longer deprecating any functions or arguments in admiral.

  • Pro: Increases stability for new users

  • Con: Increases code base to maintain for developers

    • Maintain old functions even if new function is superior
    • Now have a contract with users to apply bug fixes to old functions
  • Con: More functions in reference documents increases search time for users...confusion.

Strategy post-1.0.0

Perhaps we should continue to deprecate functions through a process of superseding and then deprecating over a three year cycle.

Proposal:

  • Functions that are superseded deprecated are given a two-year message cycle

    • Message recommends replacement function and timeline when this message will convert to a deprecation warning
  • After two-year superseded deprecated function message is reached the message turns into a 1 year deprecation function warning

  • After 1 year deprecation is reached the function is removed from the package

  • Pro: Reduces number of functions we have to maintain

  • Pro: Users are less overwhelmed with function options

  • Con: Users will have to update their code over time

  • Con: Developers have to maintain the cycle

@bms63 bms63 transferred this issue from pharmaverse/admiral Oct 7, 2024
bms63 added a commit that referenced this issue Oct 8, 2024
bms63 added a commit that referenced this issue Oct 8, 2024
bms63 added a commit that referenced this issue Oct 8, 2024
bms63 added a commit that referenced this issue Oct 8, 2024
@bms63 bms63 linked a pull request Oct 8, 2024 that will close this issue
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant