rfc 79: cdk v2.0 - change strategy on how deprecated APIs are handled #241
Conversation
Two changes to the RFC: - The branch name `next` has been updated to `v2` throughout the RFC. - The strategy on how deprecated APIs are handled has been revamped to be less manual and more automated. A one-time clean up towards the end of the project is included.
|
Moving this to draft as I work through some of the details. |
| - `cdk-build` will be updated to recognize the environment variable `CDK_BLOCK_DEPRECATIONS` and set the | ||
| `--no-deprecated` flag on the `jsii` assembler. | ||
|
|
||
| - All current usage of deprecated APIs within the CDK repository will be removed. In addition, a new check will be |
There was a problem hiding this comment.
What about things that are deprecated because we think it's a bad idea to use them, but that we don't want to remove? (One example that springs to my mind are unversioned Engines in RDS, but I'm sure there's more of them)
We need some mechanism for preserving deprecated things that we don't want to remove in v2.0.
There was a problem hiding this comment.
What's this new category of deprecated APIs?
If it's a bad idea for customers to use them, they should be removed, and customers should use the better APIs that should now be available.
There was a problem hiding this comment.
What's this new category of deprecated APIs?
If it's a bad idea for customers to use them, they should be removed, and customers should use the better APIs that should now be available.
That's not really an option, because L2s should model the entire service area of the service, even things we consider not to be good practices.
There was a problem hiding this comment.
Still not clear what you mean.
In the example you've provided, they would use the versioned engines API, no?
There was a problem hiding this comment.
Yes, but that would mean it's impossible to create an Instance/Cluster without a version, violating the above tenet.
There was a problem hiding this comment.
If the api will not be removed in the next major version is not deprecated. Maybe it's "NOT RECOMMENDED TO USE" but the term deprecated should not mean two different things. Keep it simple please
There was a problem hiding this comment.
+1 to @eladb.
It's not clear how much the IDE helps the user today anyway. In fact, we're now adding the word deprecated into the doc summary - aws/jsii@ce8c0c4
There was a problem hiding this comment.
Usually, IDEs mark the usage of something deprecated pretty clearly (here's an example in Intellij).
There was a problem hiding this comment.
If the api will not be removed in the next major version is not deprecated. Maybe it's "NOT RECOMMENDED TO USE" but the term deprecated should not mean two different things. Keep it simple please
This is not the definition of "deprecated" that I know. For example, there are APIs still available in Java that have been deprecated since the 90s.
There was a problem hiding this comment.
Usually, IDEs mark the usage of something deprecated pretty clearly (here's an example in Intellij).
That's not the case for VSCode unfortunately, which is probably the most used by our users.
For example, there are APIs still available in Java that have been deprecated since the 90s.
The fact that java decided to not remove these between major versions doesn't mean we should leave deprecated "debt" behind.
I think this is just about the semantics and I really prefer that keep the term "deprecated" reserved for APIs that will (most likely) be removed in the next major release (unless there are very unique circumstances that may require keeping it for another round). The api you are describing is not being deprecated but perhaps not recommended to use. If possible we should deprecate and remove it, if not, we should state in the docs and even add a warning if it's used.
There was a problem hiding this comment.
I think I found a situation where this strategy is going to break:
This is where we purposely call a deprecated API for backwards compatibility, and that will need to stay in.
I am okay with the general gist of this strategy, but we're going to need an exclude list of some sort.
Why not change that line to |
|
The point is: people used to be able to override a method on |
mergify
bot
dismissed
rix0rrr’s stale review
Pull request has been modified.
Two changes to the RFC:
nexthas been updated tov2throughout the RFC.The name
v2is a lot clearer on what the branch indicates than slightlymore vague
next.be less manual and more automated. A one-time clean up towards the end
of the project is included.
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache-2.0 license