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

chore(cloudfront): Removed duplicate origins in aws-cloudfront module #9326

Merged
merged 2 commits into from
Jul 29, 2020

Conversation

njlynch
Copy link
Contributor

@njlynch njlynch commented Jul 29, 2020

Prior to this change, there were both HttpOrigin and S3Origin classes in both
the aws-cloudfront and aws-cloudfront-origins module. The behaviors of the
S3Origin classes were also slightly different.

This change removes the duplication by removing the aws-cloudfront versions of
the origins.


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

Prior to this change, there were both HttpOrigin and S3Origin classes in both
the aws-cloudfront and aws-cloudfront-origins module. The behaviors of the
S3Origin classes were also slightly different.

This change removes the duplication by removing the aws-cloudfront versions of
the origins.
@njlynch njlynch requested a review from a team July 29, 2020 10:43
@njlynch njlynch self-assigned this Jul 29, 2020
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jul 29, 2020
@njlynch njlynch requested a review from eladb July 29, 2020 12:54
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 6d873e1
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Jul 29, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 9f5b0bc into master Jul 29, 2020
@mergify mergify bot deleted the njlynch/cf-remove-dupe-origins branch July 29, 2020 13:24
@skinny85
Copy link
Contributor

@njlynch I feel like we missed a bunch of BREAKING CHANGES here :(. We should remember to add them to the release notes for the next published CDK version.

@skinny85
Copy link
Contributor

Also, ReadMe changes...?

@njlynch
Copy link
Contributor Author

njlynch commented Jul 29, 2020

I feel like we missed a bunch of BREAKING CHANGES here :(. We should remember to add them to the release notes for the next published CDK version.

Whoops! I haven't had to do that yet; totally forgot about it. Can we manually add them into the release notes (and is there a mechanism so we don't forget), or could we do something weird like pushing a no-op PR/commit with just the BREAKING CHANGES notes?

Also, ReadMe changes...?

The README already only showed usages of the -origins module, so it's already consistent.

@skinny85
Copy link
Contributor

I feel like we missed a bunch of BREAKING CHANGES here :(. We should remember to add them to the release notes for the next published CDK version.

Whoops! I haven't had to do that yet; totally forgot about it. Can we manually add them into the release notes (and is there a mechanism so we don't forget), or could we do something weird like pushing a no-op PR/commit with just the BREAKING CHANGES notes?

There's no mechanism to not forget; we just have to remember to add them to the "bump" PR for the next release.

Alternatively, since forgetting is a significant risk, I'm fine with pushing an empty PR (assuming GitHub handles those) just for the commit message.

Also, ReadMe changes...?

The README already only showed usages of the -origins module, so it's already consistent.

OK. I guess that confirms the changes in this PR were the correct decision 🙂.

njlynch added a commit that referenced this pull request Jul 30, 2020
Making a small -- but useful -- README change as an excuse to capture the
breaking changes made in #9326, but that were missed in that commit message.

BREAKING CHANGE: Removed origin classes from the aws-cloudfront module.
* **aws-cloudfront:** Removed S3Origin and HttpOrigin from the aws-cloudfront
  module. Use the S3Origin and HttpOrigin classes in the aws-cloudfront-origins
  module instead.
* **aws-cloudfront:** Renamed Origin to OriginBase.
mergify bot pushed a commit that referenced this pull request Jul 30, 2020
Making a small -- but useful -- README change as an excuse to capture the
breaking changes made in #9326, but that were missed in that commit message.

BREAKING CHANGE: Removed origin classes from the aws-cloudfront module.
* **aws-cloudfront:** Removed S3Origin and HttpOrigin from the aws-cloudfront module. Use the S3Origin and HttpOrigin classes in the aws-cloudfront-origins module instead.
* **aws-cloudfront:** Renamed Origin to OriginBase.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
curtiseppel pushed a commit to curtiseppel/aws-cdk that referenced this pull request Aug 11, 2020
…aws#9326)

Prior to this change, there were both HttpOrigin and S3Origin classes in both
the aws-cloudfront and aws-cloudfront-origins module. The behaviors of the
S3Origin classes were also slightly different.

This change removes the duplication by removing the aws-cloudfront versions of
the origins.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
curtiseppel pushed a commit to curtiseppel/aws-cdk that referenced this pull request Aug 11, 2020
…9356)

Making a small -- but useful -- README change as an excuse to capture the
breaking changes made in aws#9326, but that were missed in that commit message.

BREAKING CHANGE: Removed origin classes from the aws-cloudfront module.
* **aws-cloudfront:** Removed S3Origin and HttpOrigin from the aws-cloudfront module. Use the S3Origin and HttpOrigin classes in the aws-cloudfront-origins module instead.
* **aws-cloudfront:** Renamed Origin to OriginBase.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants