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.

Sign up for GitHub

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

Jump to bottom

rfc: feature flags #5017

Closed
wants to merge 14 commits into from
Closed

rfc: feature flags #5017

wants to merge 14 commits into from

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Nov 13, 2019

Design proposal for a pattern/mechanism that will allow us to introduce
breaking capabilities which will only be applied to new projects created
by "cdk init" and won't break old projects without explicit action from the user.

NOTE: this PR is a "re-do" of the feature flags RFC which was already merged to master.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@eladb eladb added the management/rfc request for comments label Nov 13, 2019
@eladb eladb requested a review from a team November 13, 2019 20:13
@eladb eladb added the pr/do-not-merge This PR should not be merged at this time. label Nov 13, 2019
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Nov 13, 2019
@eladb eladb self-assigned this Nov 13, 2019
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

shivlaks
shivlaks reviewed Nov 14, 2019
View reviewed changes
Copy link
Contributor

@shivlaks shivlaks left a comment

Choose a reason for hiding this comment

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

Are there also implications of issue reporting, triaging issues. I'm curious what the support model would look like. Would it be as simple as adding the feature-flags stanza into the issue so bugs can be reproduced?

design/feature-flags.md Outdated Show resolved Hide resolved
design/feature-flags.md Outdated Show resolved Hide resolved
design/feature-flags.md Outdated Show resolved Hide resolved
design/feature-flags.md Outdated Show resolved Hide resolved
design/feature-flags.md Outdated Show resolved Hide resolved
@mergify
Copy link
Contributor

mergify bot commented Dec 16, 2019

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

1 similar comment
@mergify
Copy link
Contributor

mergify bot commented Dec 16, 2019

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

@eladb
Copy link
Contributor Author

eladb commented Dec 16, 2019

@shivlaks commented:

Are there also implications of issue reporting, triaging issues. I'm curious what the support model would look like. Would it be as simple as adding the feature-flags stanza into the issue so bugs can be reproduced?

Yes, that's a great point. Ideally these should be included in cdk doctor and our bug reporting issue template should request people to provide the output of cdk doctor.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

nija-at
nija-at reviewed Dec 17, 2019
View reviewed changes
Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

TBH, this is somewhat scary! I hope that we use this to the minimum.
I hope we don't get to a day where a feature flags and conditional based on these flags are not littered through our code base.

Could you provide more guidance on when feature flags should, could and must not be used, with stress on the latter? The 'duplicate stack name' is a good example of when it should be used; when should it not be used.

What is the strategy for testing new/existing features behind these flags?
It's probably fair to assume that most of the features flagged through here are going to span multiple L2s, if not multiple modules. We're going to have places in code that should've been flagged off getting missed. This is likely exacerbated by the fact that we don't run our integration tests.

design/feature-flags.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
skinny85
skinny85 reviewed Dec 17, 2019
View reviewed changes
Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

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

I like this. I'm personally not too worried we'll have 20 of these - it seems unlikely to me, and even if we do, I think that's fine.

We will definitely need this mechanism to have an upgrade story for changing how assets work for CI/CD.

design/feature-flags.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Elad Ben-Israel added 2 commits December 18, 2019 14:03
A few updates to CONTRIBUTING
2c60c8f 
- Add a disclaimer and a requirement to get an approval for using a feature flag.
- Add a prefix in the BREAKING CHANGES section.
added developer experience section
5d5e396
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@eladb eladb changed the base branch from revert/feature-flags to master December 25, 2019 09:47
@mergify
Copy link
Contributor

mergify bot commented Dec 25, 2019

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

MrArnoldPalmer
MrArnoldPalmer requested changes Dec 27, 2019
View reviewed changes
design/feature-flags.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
design/feature-flags.md Show resolved Hide resolved
@mergify
Copy link
Contributor

mergify bot commented Dec 30, 2019

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb pushed a commit to aws/aws-cdk-rfcs that referenced this pull request Jan 1, 2020
rfc: feature flags
b8350ef 
Design proposal for a pattern/mechanism that will allow us to introduce
breaking capabilities which will only be applied to new projects created
by "cdk init" and won't break old projects without explicit action from the user.

Transferred from aws/aws-cdk#5017
@eladb eladb mentioned this pull request Jan 1, 2020
Merged
@eladb
Copy link
Contributor Author

eladb commented Jan 1, 2020

Transferred this PR to the RFCs repository: aws/aws-cdk-rfcs#56

@eladb eladb closed this Jan 1, 2020
eladb pushed a commit to aws/aws-cdk-rfcs that referenced this pull request Jan 3, 2020
RFC: #55 Feature Flags (#56)
e6776a1 
Design proposal for a pattern/mechanism that will allow us to introduce breaking capabilities which will only be applied to new projects created by "cdk init" and won't break old projects without explicit action from the user.

Transferred from aws/aws-cdk#5017
Tracking issue: #55
@RomainMuller RomainMuller deleted the rfc/feature-flags branch January 3, 2020 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shivlaks shivlaks shivlaks left review comments

@nija-at nija-at nija-at left review comments

@skinny85 skinny85 skinny85 left review comments

@MrArnoldPalmer MrArnoldPalmer MrArnoldPalmer approved these changes

Assignees

@eladb eladb

Labels
contribution/core This is a PR that came from AWS. management/rfc request for comments pr/do-not-merge This PR should not be merged at this time.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@eladb @aws-cdk-automation @skinny85 @MrArnoldPalmer @nija-at @shivlaks