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: master version in one place #6410

Merged
merged 9 commits into from
Feb 24, 2020
Merged

chore: master version in one place #6410

merged 9 commits into from
Feb 24, 2020

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Feb 23, 2020

This change includes two commits:

Store the current repo version only in the root package.json in order to avoid the need to specify the version number everywhere in the codebase.

This has two benefits:

  1. Simplify the bump process: basically bump is not an update of the version in one place and CHANGELOG
  2. Remove the potential merge conflicts when merging back from "release" to "master"

In order to achieve that, I changed all package.json files to use a version marker 999.999.999 to indicate it's a local dependency. This marker is better than 0.0.0.0 since it will make sure cache is invalidated for various languages (for example, Maven will only bring in a new version if the local cache has an older version).

This change also modifies all version requirements to use fixed versions instead of caret (^) versions.

Then, we needed to change a couple of other things:

  • The bump.sh script is now very simple: it merely updates the root package.json and creates the CHANGELOG.
  • In order for CI builds to actually produce artifacts with the correct version, The script scripts/set-version.sh is executed in buildspec.yml before the build, and it uses lerna version to update the all package.json files to the actual version. This means that build artifacts that are produced in CI builds get the actual repo version, and dev builds all use 999.999.999.

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

Elad Ben-Israel added 2 commits February 23, 2020 17:51
To avoid merge conflicts when merging back a release, update the checked-in repo version number to 999.999.999.

Also, use fixed versions across the board (no carets!).
Store the current repo version only in the root pacakge.json in order to avoid the need to specify the version number everywhere in the codebase.

In order to achieve that, I changed all package.json files to use a version marker 999.999.999 to indicate it's a local dependency. This marker is better than 0.0.0.0 since it will make sure cache is invalidated for various languages (for example, Maven will only bring in a new version if the local cache has an older version).

Then, we needed to change a couple of other things:

- The bump script is now very simple. It merely updates the root package.json and creates the CHANGELOG. That's it.
- Before running a CI build (buildspec.yml), we run `scripts/set-version.sh` which uses lerna to update the version in all package.json files. This means that build artifacts that are produced in CI builds get the actual repo version, and dev builds all use 999.999.999.
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Feb 23, 2020
@eladb eladb requested a review from a team February 23, 2020 16:18
@eladb eladb changed the title chore: master version chore: master repo version only in root package.json Feb 23, 2020
@eladb eladb changed the title chore: master repo version only in root package.json chore: master version in one place Feb 23, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 24, 2020

Is this going to mess with local cdk inits?

(I mean, the answer is "yes" but the question is how bad?)

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 24, 2020

I find the 999.999.999 very comical-looking and a little distracting because of its length. Can we do something shorter like 999.0.0 (or 999.9.9 if you want more nines in there?)

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 24, 2020

Other than that I fully support this change!

@eladb
Copy link
Contributor Author

eladb commented Feb 24, 2020

I will change to 999.0.0 I see your point ;-)

Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

After discussing on Chime that peerDependencies have gotten fixed versions:

peerDependencies should be using carets. That way, the indicate a minimum version required (but not a maximum! Otherwise we're back to the bad old days of "all libraries and the customer must be using the same version of the CDK with no slack", which is not great)

Elad Ben-Israel added 3 commits February 24, 2020 16:53
Move configuration from package.json to bump.sh and change the commit title to `chore(release): vX.Y.Z`
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 2686e01
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@eladb
Copy link
Contributor Author

eladb commented Feb 24, 2020

@rix0rrr I looked into changing peer dependencies into carets and it seems like we haven't had that previously (look at the diff), so I prefer not to introduce this change right now. I also think that given our modules declare all their dependencies as fixed, a caret peer would not help to address the use case you describe (will only make it worse actually).

The current state of the world when it comes to deps is that consumers must use the same version of the AWS CDK they use for all their deps.

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 24, 2020

The current state of the world when it comes to deps is that consumers must use the same version of the AWS CDK they use for all their deps.

Okay, not part of this particular change I guess

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: d2d8594
  • 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

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 5298018
  • 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

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 5298018
  • 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

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: a57f867
  • 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

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 727335e
  • 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 Feb 24, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@mergify
Copy link
Contributor

mergify bot commented Feb 24, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@mergify mergify bot merged commit c193aac into master Feb 24, 2020
@mergify mergify bot deleted the benisrae/master-version branch February 24, 2020 21:02
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

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