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: Backport v0.25.1 changes #1952

Merged
merged 5 commits into from
Mar 5, 2019
Merged

chore: Backport v0.25.1 changes #1952

merged 5 commits into from
Mar 5, 2019

Conversation

sam-goodwin
Copy link
Contributor

Merges release v0.25.1 back into master


Pull Request Checklist

  • Testing
    • Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
    • CLI change?: coordinate update of integration tests with team
    • cdk-init template change?: coordinated update of integration tests with team
  • Docs
    • jsdocs: All public APIs documented
    • README: README and/or documentation topic updated
  • Title and Description
    • Change type: title prefixed with fix, feat will appear in changelog
    • Title: use lower-case and doesn't end with a period
    • Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
    • Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
  • Sensitive Modules (requires 2 PR approvers)
    • IAM Policy Document (in @aws-cdk/aws-iam)
    • EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
    • Grant APIs (only if not based on official documentation with a reference)

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

rix0rrr and others added 3 commits March 4, 2019 13:45
Fix issue where context passed in via command-line was ignored.

Migration behavior has been changed: context that looks like it
has been generated by context providers (keys containing ":")
will upon loading be migrated to `cdk.context.json`; other
context in `cdk.json` will be left as-is and treated as readonly.

Fixes #1911.
@sam-goodwin sam-goodwin requested review from skinny85, SoManyHs and a team as code owners March 5, 2019 03:46
@sam-goodwin sam-goodwin changed the title Merge patch release v0.25.1 into master chore: Backport v0.25.1 changes Mar 5, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 5, 2019

I think we shouldn't be doing this in general.

Git merges of cherry-picked commits are a recipe for merge conflicts.

The code is already in there, so no need to merge that back. The only reason we would do this is for the CHANGELOG. Do we really care that much about it?

@RomainMuller
Copy link
Contributor

RomainMuller commented Mar 5, 2019

I think we do care about the CHANGELOG, or else it should only contain entries for the current version. And git merge of cherry-picked commits shouldn't cause merge conflicts, since as you aptly say the changes are already there (and git can determine that).

Additionally, automatic version bumps would be tempted to re-use 0.25.1 if that version does not exist in the master tree...

@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 5, 2019

And git merge of cherry-picked commits shouldn't cause merge conflicts

Only if nothing about the lines changed (so the diff is literally nothing). If they've been moved around in the mean time, or other small changes have happened to them that a git merge with ancestry would have been able to deal with, because the ancestry of the cherry-picks has been lost now git won't be able to merge.

https://blogs.msdn.microsoft.com/oldnewthing/20180312-00/?p=98215

https://blog.founddrama.net/2013/07/git-cherry-pick/comment-page-1/

@RomainMuller RomainMuller merged commit 7b3a30b into master Mar 5, 2019
@RomainMuller RomainMuller deleted the samgood/merge-v0.25.1 branch March 5, 2019 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants