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

RFC: Sustainability of TensorFlow Addons #84

Merged
merged 4 commits into from
May 13, 2019
Merged

Conversation

dynamicwebpaige
Copy link
Contributor

@dynamicwebpaige dynamicwebpaige commented Mar 25, 2019

Comment period ends 2019-04-08

Sustainability of TensorFlow Addons

Status Proposed
Author(s) Paige Bailey (webpaige@google.com), Sean Morgan (seanmorgan@outlook.com), Yan Facai (facai.yan@gmail.com)
Sponsor Karmel Allison (karmel@google.com)
Updated 2019-03-23

Objective

TensorFlow Addons is a repository of contributions that conform to well-established API patterns, but implement new functionality not available in core TensorFlow. For TensorFlow Addons to support code that is moved from tf.contrib to addons, as well as new functionality, requirements for maintaining and retiring those submodules must be defined and enforced.

This document details requirements and responsibilities for owning a subpackage/submodule that is included in TensorFlow Addons, as well as the periodic review process for all components of Addons.

In this RFC, we are soliciting discussion regarding Addons maintainership and periodic review. This RFC discussion will help the SIG Addons team determine appropriate roles and responsibilities for proxy maintainers.

This document details requirements and responsibilities for owning a subpackage/submodule that is included in TensorFlow Addons, as well as the periodic review process for all components of Addons.

Created by Sean Morgan (@seanpmorgan), Yan Facai (@facaiy), Paige Bailey (@dynamicwebpaige).
@ewilderj ewilderj added the RFC: Proposed RFC Design Document label Mar 25, 2019
@ewilderj ewilderj changed the title RFC: Sustainability of TensorFlow Addons. RFC: Sustainability of TensorFlow Addons Mar 25, 2019
rfcs/20190308-addons-proxy-maintainership.md Outdated Show resolved Hide resolved
rfcs/20190308-addons-proxy-maintainership.md Outdated Show resolved Hide resolved
rfcs/20190308-addons-proxy-maintainership.md Outdated Show resolved Hide resolved
@bhack
Copy link
Contributor

bhack commented Mar 25, 2019

One of the main issue about addons/plugins systems is how you can quickly be aware on what addon you could rely expecially in produciton (without manually checking ticket lag, commit activity etc..).
Can we have a quick reference in the Readme.md about "quality/support" discrete levels of different submodules?
We can never have all the submodules with the same homogeneous level of maturity/support over the time. Popularity could be a good proxy but it doesn't work so well for MIA.

@facaiy
Copy link
Member

facaiy commented Mar 25, 2019

@bhack

Can we have a quick reference in the Readme.md about "quality/support" discrete levels of different submodules?

Sounds great. That's why we propose a periodic review process for all components of Addons. @seanpmorgan @dynamicwebpaige What do you think, Sean, Paige?

3. Amount of issues or bugs attributed to the code

Because we encourage TF-Addons to be used in production, we'll plan on a
graceful deprecation process, instead of a swift removal.
Copy link
Contributor

Choose a reason for hiding this comment

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

similar here, should we define what is graceful in this case. Could be too early, but potentially very valuable promises for people introducing addons in production

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this is probably better done sooner rather than later. I do think we'll reserve deprecating things unless they're truly unused in the community at least for a while.

Copy link
Contributor

Choose a reason for hiding this comment

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

explicit is better than implicit ;)

Copy link
Member

Choose a reason for hiding this comment

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

Agreed that "explicit is better than implicit".

IMHO, "deprecation" can come from different reasons, eg if the lib is short of maintenance/full of bug, or a better solution is available to solve the same problem, which makes the current API less useful.

I think API can be categorized into 3 groups: Suggested/Discouraged/Deprecated.
Suggested: Well maintained API and encourage user to use.
Discouraged: Better alternative is available, the API is kept for historical reason; or just in the waiting period to be deprecated; or the API is short of maintenance, which might cause issue in prod.
Deprecated: Use at your own risk, subject to be deleted after certain peroid.

The status change between these three is: Suggested <-> Discouraged -> Deprecated

The period between an API being marked as deprecated and being deleted can be 90 days.

  1. If addons does release monthly, there will be 2-3 releases before an API is deleted. The release notes could give user enough warning.
  2. It is not too long comparing to 180 days, nor too short as 30 days, so that people can fix their code.
  3. Google project zero uses 90 days window for security fix, which is a trade off between risk and time to respond. For more comparison of different time window, please see https://googleprojectzero.blogspot.com/search?q=90+days.

Also we should be mindful the amount of Discouraged API. They should either be fixed, or move to deprecated. Keeping Discouraged API will make long tail and cause the repo hard to maintain.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is a solid plan... would you like to submit a modification to this PR? Not sure if you have access to the branch, but @dynamicwebpaige should be able to help.

Copy link
Member

Choose a reason for hiding this comment

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

I think its fine for either you or Paige to just update the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating now! And very much like the plan for Suggested -> Discouraged -> Deprecated. 👍

3. Amount of issues or bugs attributed to the code

Because we encourage TF-Addons to be used in production, we'll plan on a
graceful deprecation process, instead of a swift removal.
Copy link

@Smokrow Smokrow Apr 7, 2019

Choose a reason for hiding this comment

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

We discussed the idea of a 2 phase deprecation process during the monthly meeting. We mark modules as "Going to be removed if nobody responds" or something similar. After half a year we do a "spring cleanup" and code that is not needed anymore will be removed. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Agree. @qlzh727 described a 3 phase system after this post that I think gives us even more flexibility.

@ewilderj
Copy link
Contributor

In terms of process, shall we consider the next SIG Addons meeting as the final review for this RFC and have it finalized and moved to "Accepted" after that meeting? @seanpmorgan ?

@dynamicwebpaige
Copy link
Contributor Author

In terms of process, shall we consider the next SIG Addons meeting as the final review for this RFC and have it finalized and moved to "Accepted" after that meeting? @seanpmorgan ?

SGTM! 👍

@ewilderj
Copy link
Contributor

ewilderj commented May 3, 2019

Per today's team call, please push an update changing status to Accepted, and I will merge

@ewilderj ewilderj added RFC: Accepted RFC Design Document: Accepted by Review and removed RFC: Proposed RFC Design Document labels May 13, 2019
@ewilderj ewilderj merged commit 9a74fde into master May 13, 2019
@ewilderj ewilderj deleted the dynamicwebpaige-patch-5 branch May 13, 2019 17:58
@ewilderj
Copy link
Contributor

Thanks everyone, congrats on the finalized RFC!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes RFC: Accepted RFC Design Document: Accepted by Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.