Skip to content

Conversation

@rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Mar 17, 2025

Describe the compatibility model, and add a table linking the CLI and library versions.


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

@rix0rrr rix0rrr requested a review from a team March 17, 2025 11:02
@github-actions github-actions bot added the p2 label Mar 17, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team March 17, 2025 11:04
@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.92%. Comparing base (f855b15) to head (76131cc).
⚠️ Report is 406 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #239      +/-   ##
==========================================
- Coverage   85.15%   84.92%   -0.24%     
==========================================
  Files         208      208              
  Lines       35711    35711              
  Branches     4636     4628       -8     
==========================================
- Hits        30410    30327      -83     
- Misses       5152     5231      +79     
- Partials      149      153       +4     
Flag Coverage Δ
suite.unit 84.92% <ø> (-0.24%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment on lines 3 to 80
Before CDK CLI `2.1000.0`, the CLI and the library were released in lockstep
with the same version number. The compatibility model was: the CDK CLI supported
CDK Apps written against the AWS Construct Library with the same version or
lower. It might have supported more versions as well, but it *at least*
always supported these.

Since CDK CLI `2.1000.0`, the compatibility between the versions depends on
the release *time*, rather than the version. The CDK CLI supports CDK apps written
against Construct Libraries *released before it*, and also the first one released
*after* the CLI is released. It might support more versions, but it *at least* supports these.

In a visual example:

```
this CLI version...
o
CLI │ │ │ │ │ │ │ │
─────┬┴─────┴┬────┴──┬────┬─┴────┬──┴──────┴┬────┴───┬──┴─────→
Library │ │ │ │ │ │ │
⋏ ⋏ ⋏ ⋏ ⋏
...supports at least these library versions
```
Copy link
Contributor

@iliapolo iliapolo Mar 17, 2025

Choose a reason for hiding this comment

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

I think this section might be unclear/overwhelming for users. It gets them thinking about when versions are released and that can be tricky. How about we replace this with just a small diagram of:

  • Construct Library uses schema to write the manifest.
  • CDK CLI uses schema to read the manifest.
  • If mismatch --> error

Given the table below, I think this will be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It gets users thinking about when versions are released and that can be tricky

Can you tell me what is tricky? It's objective information that can be easily (albeit perhaps a bit laboriously) obtained.

If the trickiness is:

I can't tell from the version number alone what the compatibility is and now I need to look up information elsewhere.

Then yes, but that is the nature of the beast.

What I wanted to do here is to state the general rule people can use to determine compatibility. In addition to this rule, they can consult the table.

It seems like you are saying "don't state a general rule at all, just give the table and tell people to always look it up in the Markdown table, rather than either in the Markdown table or on npmjs".

I think that is independent of a diagram that shows the Cloud Assembly schema, which we could do but I was also trying to keep the schema out of it as much as possible (it's an implementation detail after all) and just focus on the 2 main components.

Copy link
Contributor

@iliapolo iliapolo Mar 17, 2025

Choose a reason for hiding this comment

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

Can you tell me what is tricky?

My concern is that this general rule will get people to try and lookup when their CLI version was released, when their Lib version was released, and compare. I would like to avoid that because its a lot of steps. Given they cannot infer this information from version numbers alone, I think this will happen often.

So if we are sending them anywhere, I would prefer it to be the table below only, instead of looking up release dates on github or npm.

Copy link
Contributor

@iliapolo iliapolo Mar 17, 2025

Choose a reason for hiding this comment

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

(it's an implementation detail after all)

True, but I think at this point its has already become more than that (maybe because it changes more often than we thought?) I think folks would benefit from understanding which component uses it and when.

I agree its not necessarily related, but I think it may be more helpful.

COMPATIBILITY.md Outdated
Comment on lines 36 to 45
| Schema Version | CLI Version | Library version |
|----------------|-------------|-----------------|
| 39 | 2.174.0 | 2.174.0 |
| 40 | 2.1000.0 | 2.182.0 |
| 41 | 2.1003.0 | (not used yet) |

How to use the table above: to find out what version of the CDK CLI you need
to deploy your CDK App written against a particular library version, find
your library version in the right-hand column and then see the minimum version
of the CDK CLI you need in the middle column. No newline at end of file
Copy link
Contributor

@iliapolo iliapolo Mar 17, 2025

Choose a reason for hiding this comment

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

Suggested change
| Schema Version | CLI Version | Library version |
|----------------|-------------|-----------------|
| 39 | 2.174.0 | 2.174.0 |
| 40 | 2.1000.0 | 2.182.0 |
| 41 | 2.1003.0 | (not used yet) |
How to use the table above: to find out what version of the CDK CLI you need
to deploy your CDK App written against a particular library version, find
your library version in the right-hand column and then see the minimum version
of the CDK CLI you need in the middle column.
| Schema Version | CLI Version | Library version |
|----------------|-------------|-----------------|
| 39 | 2.174.0 | >=2.174.0 <2.182.0 |
| 40 | 2.1000.0 | >=2.182.0 |
| 41 | 2.1003.0 | (not used yet) |
How to use the table above: to find out what version of the CDK CLI you need, locate
your library version in the right-hand column, the CDK CLI version corresponding to it, is the minimum version you should use.

How do we keep this up to date?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do we keep this up to date?

Currently manually. Of course we should try and automate its generation. I don't have good ideas for doing that just yet and scripting it will be a significant bit of work. I don't want to gate publishing this information on having an answer to that question.

As for the table, if you want to be even more technically correct we should be adding range operators to the middle column as well:

| Schema Version | CLI Version | Library version |
|----------------|-------------|-----------------|
|  39            | >= 2.174.0     | >=2.174.0 <2.182.0         |
|  40            | >= 2.1000.0    | >=2.182.0         |
|  41            | >= 2.1003.0    | (not used yet)  |

But this adds to the visual noise. I think people will be able to infer those bounds pretty naturally, don't you think?

Copy link
Contributor Author

@rix0rrr rix0rrr Mar 17, 2025

Choose a reason for hiding this comment

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

How do we keep this up to date?

Currently manually. Of course we should try and automate its generation. I don't have good ideas for doing that just yet and scripting it will be a significant bit of work. I don't want to gate publishing this information on having an answer to that question.

As for the table, if we want to be even more technically correct we should be adding range operators to the middle column as well:

| Schema Version | CLI Version | Library version |
|----------------|-------------|-----------------|
|  39            | >= 2.174.0     | >=2.174.0 <2.182.0         |
|  40            | >= 2.1000.0    | >=2.182.0         |
|  41            | >= 2.1003.0    | (not used yet)  |

But this adds to the visual noise. I think people will be able to infer those bounds pretty naturally, don't you think? I don't think we need them.

Copy link
Contributor

Choose a reason for hiding this comment

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

But this adds to the visual noise. I think people will be able to infer those bounds pretty naturally, don't you think? I don't think we need them.

It does add some clutter, but its also familiar noise...I think it will help. Maybe just add a "all versions below should be read as minimum version numbers", or something?

Copy link
Contributor

@iliapolo iliapolo Mar 17, 2025

Choose a reason for hiding this comment

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

I don't want to gate publishing this information on having an answer to that question.

Thats fine. Would be good to at least reference this table in a place developers stumble upon when making schema changes, but I can' think of a good place because the version bump is automatic. If you can think of a place...if not, oh well.

@rix0rrr rix0rrr added the pr/exempt-integ-test Skips the integ test steps if set. label Mar 17, 2025
@iliapolo iliapolo disabled auto-merge March 17, 2025 12:48
@iliapolo
Copy link
Contributor

Dequeued to address comments at will.

Describe the compatibility model, and add a table linking
the CLI and library versions.
@rix0rrr rix0rrr force-pushed the huijbers/cli-compatibility-doc branch from 25d3e07 to 76131cc Compare March 17, 2025 13:19
@rix0rrr rix0rrr enabled auto-merge March 17, 2025 13:32
@rix0rrr rix0rrr added this pull request to the merge queue Mar 17, 2025
Merged via the queue into main with commit a1fbefe Mar 17, 2025
11 checks passed
@rix0rrr rix0rrr deleted the huijbers/cli-compatibility-doc branch March 17, 2025 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p2 pr/exempt-integ-test Skips the integ test steps if set.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants