-
Notifications
You must be signed in to change notification settings - Fork 578
SIG Addons: Procedure and RFC template for Addons migrations #241
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
Conversation
* What are the use cases for the addon? | ||
* OSS usage, H5 Index, etc. | ||
|
||
## Historical Information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how it is easy to have a quick query but for stability we could ask how many bugs was handled in the last (x) month(s).
### In-Progress & Previous Migrations: | ||
https://github.com/tensorflow/addons/projects/2/ | ||
|
||
### Process |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be quite complicated for cases like ImageProjectiveTransformV2
.
As we could have an upstream request where the sponsor is already in the TF core team I don't think this double pass is plausible for this specific case.
EDIT:
Here I meant that if there upstream proposer and the sponsor overlaps (in the case the promotion activity it is started by a TF team member) I don't think that we will like to have this double pass evaluation (on the SIG side and then here with the RFC). So I suppose that we want to define a fast track process in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be helping external contribution for the migration only, not internal contribution for the migration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tanzhenyu Do you mean that you want a different process for Google internals?
[Keras Governance](https://github.com/keras-team/governance) | ||
|
||
4. A sponsor from the TF/Keras team must agree to shepard the transition. | ||
* If no sponsor is obtained after 45 days the RFC will be rejected and will remain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am ok to put a number here but I think that we need a realistic evaluation by the TF team. Is 45 days something realistic for the internal process?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could align to 1 month. See https://github.com/tensorflow/community/blob/master/governance/TF-RFCs.md#rfc-process
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, would like to align here but 45 days ensures we overlap with a TFA monthly meeting and lets us get help from TF internal team members to help find sponsors. @theadactyl @ewilderj is there anyway we could make 45 days the standard amount of time for RFC sponsorship? I imagine this would be useful for multiple SIGs that have monthly meetings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is the same if we could extend the standard to 45 days is ok
sigs/addons/MIGRATION_TO_CORE.md
Outdated
warning will be added and will be removed from TFA after 2 releases. | ||
|
||
|
||
### Criteria for Migration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ok for me on the SIG side. I just want to wait if TF team could put some quantitative metrics here.
Cause the sponsor define the bandwidth of the maintainership but having some quantitative metrics will make the process more transparent (if possible).
sigs/addons/MIGRATION_TO_CORE.md
Outdated
warning will be added and will be removed from TFA after 2 releases. | ||
|
||
|
||
### Criteria for Migration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...and someone on the TensorFlow team agrees to take over maintenance of the API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm so I think this is a bit of a chicken and egg problem. The sponsor will often be determined after the RFC has been created. I've renamed this as Criteria for Migration RFC
PTAL and let me know if you think that makes more sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! We need to relax the requirements for the alias.
* If the transition will impact tf-core and Keras then submit the RFC to | ||
[TensorFlow Community](https://github.com/tensorflow/community) | ||
* Additions which only subclass Keras APIs should submit their migration proposals to | ||
[Keras Governance](https://github.com/keras-team/governance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new Keras website is online. I don't know if we need to duplicate/fork this template also in Keras governance and toadd something to the Keras SIG new webpage.
We are in the special case here where a SIG is an upstream of another SIG.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seanpmorgan We have other new guidelines for the Keras upstream: https://github.com/keras-team/governance/blob/master/keras_api_design_guidelines.md
Per SIG discussion with TF team, we've determined that it is infeasible for an external SIG to handle the migration process. In the future core TF and Keras member will handle the migration / deprecations: |
Tried to incorporate the discussion points in #239 into documentation. Closes #239 once agreed upon.
The goal of this PR is to standardize how Addons components will be migrated going forward. After the RFC template and and Procedure are agreed upon we will submit the proposal for GELU migration.
cc @karmel @alextp @bhack for feedback