From c11533ed98233133f2440b570074d31c51cb8d2c Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Tue, 19 Mar 2019 22:19:05 -0700 Subject: [PATCH 1/9] desing proposal template --- docs/design_proposals/README.md | 27 ++++++ .../design-proposal-template.md | 83 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 docs/design_proposals/README.md create mode 100644 docs/design_proposals/design-proposal-template.md diff --git a/docs/design_proposals/README.md b/docs/design_proposals/README.md new file mode 100644 index 00000000000..cc54285524a --- /dev/null +++ b/docs/design_proposals/README.md @@ -0,0 +1,27 @@ +# Design Proposals process + +Hello Contributors! + +This document describes the process for proposing a new feature or making any +big code changes to `skaffold`. + +Having a proposal, will likely reduce the back and forth between the contributor +and the core team. It also makes sure, each new feature or a big change has a +design review. + +For any new feature, config or big changes, please add a design proposal document +as described in [Design Proposal Template](./design-proposal-template.md). + +Once you create a PR with the proposal, someone from the core team will be +assigned as a design sheppard. The role of the design sheppard will be to make +sure, + +1. The feature/change is within Skaffold Philosophy and not a one off + solution for a specific use case. +2. The feature/change scope is well defined. +3. When changing any existing feature, the implementation plan adheres to + [skaffold deprecation policy](./../../deprecation-policy.md) + +Once the proposals in a reasonale shape, we can discuss it in Skaffold bi-weekly +meeting to address any open concerns, and reach to a decision i.e. accept or +punt. diff --git a/docs/design_proposals/design-proposal-template.md b/docs/design_proposals/design-proposal-template.md new file mode 100644 index 00000000000..55b84b40517 --- /dev/null +++ b/docs/design_proposals/design-proposal-template.md @@ -0,0 +1,83 @@ +# Title + +* Author(s): \ +* Design Shepherd: As mentioned in the document here. +* Date: \ + +## Background + +In this section, please mention and describe the new feature, re-design +or re-factor. + +Please provide an rationale covering following points: + +1. Why is this required? +2. If its re-design, What are cons with current implementation? +3. Are there any another work-around and if yes, why not keep using it. + +Here is an example snippet for a new feature: + +___ +Currently, skaffold config supports `artifact.sync` as a way to sync files +directly to pods. So far, artifact sync requires a specification of sync +patterns like + +```yaml +sync: + '*.js': app/ +``` + +This is error prone and unnecessarily hard to use, because the destination is +already contained in the Dockerfile for docker build. (see #1166, #1581). +In addition, the syncing needs to handle special cases for globbing and often +requires a long list of sync patterns (#1807) +___ + +## Design + +Please describe your solution. Please list any: + +* new config changes +* interface changes +* design assumptions + +For a new config change, please mention: + +* If its a backward compatible config change +* If its a backward compatible config change, is there a migration path? + +### Open Issues/Question + +Please list any open questions here in the format. + +**\** + +Resolution: Please list the resolution if resolved during the design process or +specify __Not Yet Resolved__ + +## Implementation plan +We have identified, huge PRs go unnoticed for a long time. Small incremental +changes get reviewed faster and also easier for reviewers. + +For a design feature, list a summary of tasks breakdown for e.g.: +For the example desing proposal to infer artifact sync, some of the smaller task +could be: +___ + +1. Add new config key `infer` to `artifact.sync` and test schema validation. +2. Add inference logic for docker and examples. +3. Support both `infer` and user defined map with precedence rules implemented. +4. Finally, support builder plugins to add sync patterns. +___ + +For re-factor proposal identify smaller changes like: +___ + +1. Add new package skeleton. and any new objects with no functionality. +2. Move code from old place to newly added objects. + +___ + +## Integration test plan + +Please describe what new test cases are you going to consider. From f006a766b857b5bbb845d264dc2e23b178094ab8 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 20 Mar 2019 10:43:00 -0700 Subject: [PATCH 2/9] code review changes --- docs/design_proposals/README.md | 4 ++-- .../design-proposal-template.md | 21 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/docs/design_proposals/README.md b/docs/design_proposals/README.md index cc54285524a..f83cfe4ab2b 100644 --- a/docs/design_proposals/README.md +++ b/docs/design_proposals/README.md @@ -13,7 +13,7 @@ For any new feature, config or big changes, please add a design proposal documen as described in [Design Proposal Template](./design-proposal-template.md). Once you create a PR with the proposal, someone from the core team will be -assigned as a design sheppard. The role of the design sheppard will be to make +assigned as a design shepherd. The role of the design shepherd will be to make sure, 1. The feature/change is within Skaffold Philosophy and not a one off @@ -22,6 +22,6 @@ sure, 3. When changing any existing feature, the implementation plan adheres to [skaffold deprecation policy](./../../deprecation-policy.md) -Once the proposals in a reasonale shape, we can discuss it in Skaffold bi-weekly +Once the proposal is in a reasonale shape, we can discuss it in Skaffold bi-weekly meeting to address any open concerns, and reach to a decision i.e. accept or punt. diff --git a/docs/design_proposals/design-proposal-template.md b/docs/design_proposals/design-proposal-template.md index 55b84b40517..2fbb6b9b2ba 100644 --- a/docs/design_proposals/design-proposal-template.md +++ b/docs/design_proposals/design-proposal-template.md @@ -1,8 +1,12 @@ # Title * Author(s): \ -* Design Shepherd: As mentioned in the document here. +* Design Shepherd: \ + + If you are already working with someone mention their name. + If not, please leave this empty, it will be assigned to a core team member. * Date: \ +* Status: [Draft/Reviewed/Complete] ## Background @@ -13,7 +17,8 @@ Please provide an rationale covering following points: 1. Why is this required? 2. If its re-design, What are cons with current implementation? -3. Are there any another work-around and if yes, why not keep using it. +3. Is there any another work-around and if yes, why not keep using it. +4. Mention related issues, if there are any.**** Here is an example snippet for a new feature: @@ -43,8 +48,10 @@ Please describe your solution. Please list any: For a new config change, please mention: -* If its a backward compatible config change -* If its a backward compatible config change, is there a migration path? +* If its a backward compatible config change ? +* If the answer to above question is yes, what would be the deprecation policy? + See [deprecation-policy](./../../deprecation-policy.md#how-do-we-deprecate-things) + requirements. ### Open Issues/Question @@ -68,15 +75,9 @@ ___ 2. Add inference logic for docker and examples. 3. Support both `infer` and user defined map with precedence rules implemented. 4. Finally, support builder plugins to add sync patterns. -___ -For re-factor proposal identify smaller changes like: ___ -1. Add new package skeleton. and any new objects with no functionality. -2. Move code from old place to newly added objects. - -___ ## Integration test plan From 43f7c29c0700e06a7e6e7d41431b89980f32175c Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 20 Mar 2019 13:32:50 -0700 Subject: [PATCH 3/9] fix spell mistakes --- docs/design_proposals/design-proposal-template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design_proposals/design-proposal-template.md b/docs/design_proposals/design-proposal-template.md index 2fbb6b9b2ba..35365d69e5b 100644 --- a/docs/design_proposals/design-proposal-template.md +++ b/docs/design_proposals/design-proposal-template.md @@ -18,7 +18,7 @@ Please provide an rationale covering following points: 1. Why is this required? 2. If its re-design, What are cons with current implementation? 3. Is there any another work-around and if yes, why not keep using it. -4. Mention related issues, if there are any.**** +4. Mention related issues, if there are any. Here is an example snippet for a new feature: From ab6f7b5e8020092ec86f80324a8611b83c22cb8b Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Mon, 25 Mar 2019 13:52:57 -0700 Subject: [PATCH 4/9] address nkubala's code review comments --- docs/design_proposals/README.md | 20 +++++++------- .../design-proposal-template.md | 26 +++++++++---------- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/docs/design_proposals/README.md b/docs/design_proposals/README.md index f83cfe4ab2b..ab019145c0f 100644 --- a/docs/design_proposals/README.md +++ b/docs/design_proposals/README.md @@ -1,27 +1,27 @@ -# Design Proposals process +# Design Proposal Process Hello Contributors! This document describes the process for proposing a new feature or making any big code changes to `skaffold`. -Having a proposal, will likely reduce the back and forth between the contributor -and the core team. It also makes sure, each new feature or a big change has a -design review. +Submitting a proposal before a pull request, will likely reduce the back and +forth between the contributor and the core team. A proposal also ensures, each +new feature or a big change has a design review. For any new feature, config or big changes, please add a design proposal document as described in [Design Proposal Template](./design-proposal-template.md). Once you create a PR with the proposal, someone from the core team will be assigned as a design shepherd. The role of the design shepherd will be to make -sure, +sure: -1. The feature/change is within Skaffold Philosophy and not a one off - solution for a specific use case. +1. The feature/change is within the Skaffold roadmap and the team's general + philosophy for the tool and not a one off solution for a specific use case. 2. The feature/change scope is well defined. 3. When changing any existing feature, the implementation plan adheres to [skaffold deprecation policy](./../../deprecation-policy.md) -Once the proposal is in a reasonale shape, we can discuss it in Skaffold bi-weekly -meeting to address any open concerns, and reach to a decision i.e. accept or -punt. +Once the proposal has been approved, we can move discussions to our bi-weekly +meetings to address any open concerns,and to reach a final decision on whether +or not to accept the feature or change. diff --git a/docs/design_proposals/design-proposal-template.md b/docs/design_proposals/design-proposal-template.md index 35365d69e5b..ef4ae4f543c 100644 --- a/docs/design_proposals/design-proposal-template.md +++ b/docs/design_proposals/design-proposal-template.md @@ -10,14 +10,14 @@ ## Background -In this section, please mention and describe the new feature, re-design -or re-factor. +In this section, please mention and describe the new feature, redesign +or refactor. -Please provide an rationale covering following points: +Please provide a brief explanation for the following questions: 1. Why is this required? -2. If its re-design, What are cons with current implementation? -3. Is there any another work-around and if yes, why not keep using it. +2. If this is a redesign, what are the drawbacks of the current implementation? +3. Is there any another work-around, and if so, what are its drawbacks? 4. Mention related issues, if there are any. Here is an example snippet for a new feature: @@ -48,10 +48,9 @@ Please describe your solution. Please list any: For a new config change, please mention: -* If its a backward compatible config change ? -* If the answer to above question is yes, what would be the deprecation policy? - See [deprecation-policy](./../../deprecation-policy.md#how-do-we-deprecate-things) - requirements. +* Is it backwards compatible? If not, what is the deprecation policy? + Refer to the [deprecation policy requirements.](./../../deprecation-policy.md#how-do-we-deprecate-things) + for details) ### Open Issues/Question @@ -63,12 +62,11 @@ Resolution: Please list the resolution if resolved during the design process or specify __Not Yet Resolved__ ## Implementation plan -We have identified, huge PRs go unnoticed for a long time. Small incremental -changes get reviewed faster and also easier for reviewers. +As a team, we've noticed that larger PRs can go unreviewed for long periods of +time. Small incremental changes get reviewed faster and also easier for reviewers. For a design feature, list a summary of tasks breakdown for e.g.: -For the example desing proposal to infer artifact sync, some of the smaller task -could be: +For the example artifact sync proposal, some of the smaller tasks could be: ___ 1. Add new config key `infer` to `artifact.sync` and test schema validation. @@ -81,4 +79,4 @@ ___ ## Integration test plan -Please describe what new test cases are you going to consider. +Please describe what new test cases you are going to consider. From 551dbe3b64e0b9034c3883a0ee7aeaf76011742f Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Tue, 26 Mar 2019 09:52:37 -0700 Subject: [PATCH 5/9] few nits --- docs/design_proposals/README.md | 4 ++-- docs/design_proposals/design-proposal-template.md | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/design_proposals/README.md b/docs/design_proposals/README.md index ab019145c0f..41458626a3d 100644 --- a/docs/design_proposals/README.md +++ b/docs/design_proposals/README.md @@ -5,7 +5,7 @@ Hello Contributors! This document describes the process for proposing a new feature or making any big code changes to `skaffold`. -Submitting a proposal before a pull request, will likely reduce the back and +Submitting a proposal before a pull request will likely reduce the back and forth between the contributor and the core team. A proposal also ensures, each new feature or a big change has a design review. @@ -20,7 +20,7 @@ sure: philosophy for the tool and not a one off solution for a specific use case. 2. The feature/change scope is well defined. 3. When changing any existing feature, the implementation plan adheres to - [skaffold deprecation policy](./../../deprecation-policy.md) + [skaffold deprecation policy](./../../deprecation-policy.md). Once the proposal has been approved, we can move discussions to our bi-weekly meetings to address any open concerns,and to reach a final decision on whether diff --git a/docs/design_proposals/design-proposal-template.md b/docs/design_proposals/design-proposal-template.md index ef4ae4f543c..ca4030346c7 100644 --- a/docs/design_proposals/design-proposal-template.md +++ b/docs/design_proposals/design-proposal-template.md @@ -33,9 +33,9 @@ sync: ``` This is error prone and unnecessarily hard to use, because the destination is -already contained in the Dockerfile for docker build. (see #1166, #1581). +already contained in the Dockerfile for docker build (see #1166, #1581). In addition, the syncing needs to handle special cases for globbing and often -requires a long list of sync patterns (#1807) +requires a long list of sync patterns (#1807). ___ ## Design @@ -50,7 +50,7 @@ For a new config change, please mention: * Is it backwards compatible? If not, what is the deprecation policy? Refer to the [deprecation policy requirements.](./../../deprecation-policy.md#how-do-we-deprecate-things) - for details) + for details. ### Open Issues/Question @@ -63,7 +63,8 @@ specify __Not Yet Resolved__ ## Implementation plan As a team, we've noticed that larger PRs can go unreviewed for long periods of -time. Small incremental changes get reviewed faster and also easier for reviewers. +time. Small incremental changes get reviewed faster and are also easier for +reviewers. For a design feature, list a summary of tasks breakdown for e.g.: For the example artifact sync proposal, some of the smaller tasks could be: From e28c2136e2feefe10446db0ca35a06256d0e5ba8 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Tue, 26 Mar 2019 12:46:13 -0700 Subject: [PATCH 6/9] Add note on large in impact and large in size needs Design proposal --- docs/design_proposals/README.md | 20 +++++++++++++------ .../design-proposal-template.md | 2 +- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/docs/design_proposals/README.md b/docs/design_proposals/README.md index 41458626a3d..fef2cdcee09 100644 --- a/docs/design_proposals/README.md +++ b/docs/design_proposals/README.md @@ -3,20 +3,28 @@ Hello Contributors! This document describes the process for proposing a new feature or making any -big code changes to `skaffold`. +large code changes to `skaffold`. By large we mean, large in impact or large in +size. + +An example for large impact changes would be: + +1. Introduce Templating, or +2. Arbitrary command execution in certain places + +These could be small code changes but large in impact. Submitting a proposal before a pull request will likely reduce the back and -forth between the contributor and the core team. A proposal also ensures, each -new feature or a big change has a design review. +forth between the contributor and the core team. A proposal also ensures that +each new feature or a large change has a design review. -For any new feature, config or big changes, please add a design proposal document +For any new feature, config or large change, please add a design proposal document as described in [Design Proposal Template](./design-proposal-template.md). -Once you create a PR with the proposal, someone from the core team will be +Once you create a PR with the proposal, one of the maintainers will be assigned as a design shepherd. The role of the design shepherd will be to make sure: -1. The feature/change is within the Skaffold roadmap and the team's general +1. The feature/change is aligned with the Skaffold roadmap and the team's general philosophy for the tool and not a one off solution for a specific use case. 2. The feature/change scope is well defined. 3. When changing any existing feature, the implementation plan adheres to diff --git a/docs/design_proposals/design-proposal-template.md b/docs/design_proposals/design-proposal-template.md index ca4030346c7..adebc16b4f9 100644 --- a/docs/design_proposals/design-proposal-template.md +++ b/docs/design_proposals/design-proposal-template.md @@ -17,7 +17,7 @@ Please provide a brief explanation for the following questions: 1. Why is this required? 2. If this is a redesign, what are the drawbacks of the current implementation? -3. Is there any another work-around, and if so, what are its drawbacks? +3. Is there any another workaround, and if so, what are its drawbacks? 4. Mention related issues, if there are any. Here is an example snippet for a new feature: From 5ed5aa33855f4cf6def102bd6a52e1d0ba78a8a2 Mon Sep 17 00:00:00 2001 From: Balint Pato Date: Mon, 1 Apr 2019 11:27:08 -0700 Subject: [PATCH 7/9] Update docs/design_proposals/README.md Co-Authored-By: tejal29 --- docs/design_proposals/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design_proposals/README.md b/docs/design_proposals/README.md index fef2cdcee09..c7f2e3b6f43 100644 --- a/docs/design_proposals/README.md +++ b/docs/design_proposals/README.md @@ -8,7 +8,7 @@ size. An example for large impact changes would be: -1. Introduce Templating, or +1. Introduce templating, or 2. Arbitrary command execution in certain places These could be small code changes but large in impact. From ba8bd7e4fccba2b5008129451f02cdde9ff346c8 Mon Sep 17 00:00:00 2001 From: Balint Pato Date: Mon, 1 Apr 2019 11:27:29 -0700 Subject: [PATCH 8/9] Update docs/design_proposals/README.md Co-Authored-By: tejal29 --- docs/design_proposals/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design_proposals/README.md b/docs/design_proposals/README.md index c7f2e3b6f43..2b77aeada02 100644 --- a/docs/design_proposals/README.md +++ b/docs/design_proposals/README.md @@ -6,7 +6,7 @@ This document describes the process for proposing a new feature or making any large code changes to `skaffold`. By large we mean, large in impact or large in size. -An example for large impact changes would be: +Examples for large impact changes would be: 1. Introduce templating, or 2. Arbitrary command execution in certain places From b3f9a7819b448552513f4fa728e466091d3ecb03 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Mon, 1 Apr 2019 15:00:12 -0700 Subject: [PATCH 9/9] add some more info on status --- docs/design_proposals/design-proposal-template.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/design_proposals/design-proposal-template.md b/docs/design_proposals/design-proposal-template.md index adebc16b4f9..c5276707bb6 100644 --- a/docs/design_proposals/design-proposal-template.md +++ b/docs/design_proposals/design-proposal-template.md @@ -6,7 +6,20 @@ If you are already working with someone mention their name. If not, please leave this empty, it will be assigned to a core team member. * Date: \ -* Status: [Draft/Reviewed/Complete] +* Status: [Reviewed/Cancelled/Under implementation/Complete] + +Here is a brief explanation of the Statuses + +1. Reviewed: The proposal PR has been accepted, merged and ready for + implementation. +2. Under implementation: An accepted proposal is being implemented by actual work. + Note: The design might change in this phase based on issues during + implementation. +3. Cancelled: During or before implementation the proposal was cancelled. + It could be due to: + * other features added which made the current design proposal obsolete. + * No longer a priority. +4. Complete: This feature/change is implemented. ## Background