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

Release a major version when any deprecations are removed #512

Closed

Conversation

Kerrick
Copy link

@Kerrick Kerrick commented Jul 8, 2019

Rendered


This was put together after a discussion on the Ember Community Discord. Thanks to @mixonic, @jenweber, @NullVoxPopuli, @acorncom, @rwjblue, and @samselikoff for their feedback as I discussed the problem and drafted the RFC!

>
> Thus, Chandra feels safe setting `"ember-source": "^3.0.0"` in her package.json, comforted by the presence of SemVer to keep her application safe. She deploys her company's application on June 28, 2018. Her app runs on v3.2.2 and all is well. In her `beforeModel` hook, she reads `transition.targetName` (found in [the official Ember documentation](http://api.emberjs.com/ember/3.10/classes/Route/methods?anchor=resetController )) and `transition.queryParams` ([quite](http://shotgundebugging.blogspot.com/2019/02/impersonation-in-emberjs-elegantly.html) well [documented](https://codeandtechno.com/posts/user-impersonation-ember-simple-auth-doorkeeper/) by [community](https://discuss.emberjs.com/t/getting-query-params-and-segment-values-from-parent-route/6628/6) blog [posts]( https://www.tilcode.com/tag/ember-queryparams-tutorial/) and [answers](https://stackoverflow.com/a/26706095) all [over](https://stackoverflow.com/a/43310476)).
>
> Chandra doesn't need to deploy again for a full year, as the application was developed perfectly from the start. But on July 3, 2019 she hears from a colleague that the latest 3.X version of Ember has some performance improvements that'll really help during the holiday weekend rush. Comforted by SemVer, she upgrades to v3.11.0 and deploys on the way out the door. Little does she know, [her production application broke](https://github.com/emberjs/ember.js/pull/17843)!
Copy link
Member

Choose a reason for hiding this comment

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

as the application was developed perfectly from the start

😜 INDEED!


In the current process, Intimate API changes already get deprecation warnings in some scenarios. This proposal does not suggest any changes to that process. Instead, it suggests that the outcome of the answer, "yes, this needs a deprecation," changes from, "so don't remove it until the next minor release after an LTS," to "so trigger a major version change when it releases."

This _will_ mean more major versions get released than today; but since Editions now drive paradigm shifts and marketing efforts, it shouldn't greatly affect how people talk about "what's new in Ember."
Copy link
Member

Choose a reason for hiding this comment

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

This will mean more major versions get released than today

I don't think this is necessarily true (though IMHO it's fine if it is true). I think this may well be the forcing function for "svelte" which has long been discussed (and is internally supported in the codebase) but is not actually usable by apps today.

The rough pitch for svelte is that deprecated codepaths are flagged (nearly identically to new features!), and the application would declare which version of Ember it is compatible with "deprecation free". This allows the build system to remove all deprecated code paths for features that the application has declared that it doesn't use. This dramatically reduces the "need" to remove deprecations since it goes from "we have to remove this to shave bytes" to "we generally would like to remove this so we can have cleaner code in the repo".


This _will_ mean more major versions get released than today; but since Editions now drive paradigm shifts and marketing efforts, it shouldn't greatly affect how people talk about "what's new in Ember."

This proposal also does not suggest any changes to the community's current vocabulary. Intimate APIs are still Intimate APIs, and are considered different from Public APIs. Like today, not all Intimate API changes would warrant a deprecation warning (and therefore a major version change). Intimate APIs can still be taught as a thing to be avoided. The big change here is that in today's world, those who choose not to avoid Intimate APIs are punished by minor version upgrades. In the proposed world, they instead have to deal with breaking changes in a major version but everybody else gets that major version "for free."
Copy link
Member

Choose a reason for hiding this comment

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

This seems a tad "muddy" to me, there are three types of APIs:

  • Public APIs - any API documented as public; these already can't be removed until a major version bump
  • Private APIs - any API that is not explicitly marked as "public" should be considered private; these can be removed at will, and are not considered "breaking" in the least bit
  • Intimate APIs - this one is hard to define 😸; these would be Private APIs that have enough community usage to force us to think carefully about removing arbitrarily

Specifically, sections like:

Like today, not all Intimate API changes would warrant a deprecation warning (and therefore a major version change)

I think this is slightly wrong, if an API is "intimate" it probably does need a deprecation warning. I'd rephrase this to be something like:

Like today, not all private API changes would warrant a deprecation

@rwjblue
Copy link
Member

rwjblue commented Jul 9, 2019

Deprecations are added in order to be helpful to folks that might be using a given API. They are definitely not something that is "required" at all for private APIs. Adding a policy that intentionally penalizes folks for trying to be helpful seems like the complete opposite direction that you are trying to push for, and would almost certainly result in fewer deprecations and therefore more pain for addon and app authors.

@wycats
Copy link
Member

wycats commented Jul 9, 2019

Thanks for the well-considered RFC!

You're right that the "intimate API" policy is fairly unusual. It can be difficult for people to understand unless they're deeply involved with the Ember planning process.

In a way, this RFC proposes that we have a process to upgrade private APIs to public APIs instead of the compromise policy we have now.

This feels like a reasonable approach to me, and one that I would support.

This RFC (as written), suggests that we would never issue deprecations for private APIs, because the act of issuing a deprecation means that an API is public. This might be going too far.

I would support replacing our current Intimate API policy with a process for marking a private API as public. I would also support retaining the ability to deprecate private APIs.

@rwjblue
Copy link
Member

rwjblue commented Jul 9, 2019

The harder part might be teaching core team members and other heavily-enfranchised individuals.

There is a specific tone here that I find troubling. The onus of this RFC absolutely falls on the project maintainers and core team members, and this phrasing makes it seem that the fundamental goals of core team members are not aligned with whats good for the users of the project. If that is true then we have a very big problem.

@runspired
Copy link
Contributor

I have three concerns:

  1. I suspect this encourages more private API usage
  2. how this interplays with lockstep may cause release cadence issues for other teams
  3. it suggests that "major versions for deprecation cleanup" as an alternative, but practically that is the only real solution. Were we instead to have any sort of "every X" version or worse "every Nth from when a new deprecation is introduced" we would find ourselves with major versions for most releases. This RFC should recommend a cadence for deprecation cleanup.

@Kerrick
Copy link
Author

Kerrick commented Jul 10, 2019

This RFC (as written), suggests that we would never issue deprecations for private APIs, because the act of issuing a deprecation means that an API is public. This might be going too far.

I do not want to suggest anything that makes it less likely for a private/intimate API to become deprecated. If the RFC wasn't clear on this point, I'd love some advice on how to make it clear.

Essentially, I want to introduce no changes to process besides a change to whether the Minor or Major version number gets bumped for a given release -- because now with editions, going from v3.X to v4.0 needs to be no different from going from v3.X to v3.X+1.

Things I didn't mean to imply with this RFC:

  • Delaying the removal of a deprecated API.
  • Changing how decisions are made on which private/intimate APIs to deprecate
  • Changing whether deprecations are added for private/intimate APIs.
  • Changing the six-week release process.
  • Changing the four-release LTS process.
  • Making it less likely for a private/intimate API that would currently get a deprecation, to get a deprecation.
  • Making it harder for the Ember Core Team to move Ember forward.
  • Making it harder for the Ember community to develop and publish addons.

...However, the ultimate goal of my RFC is this: to make upgrading any number of minor versions in the same major version safe for most real-world users. So if you think any of the things in my list above are good, desired, and/or necessary to achieve that goal, I'd be glad to modify the RFC to include them.

@runspired
Copy link
Contributor

I think maybe I should clarify the points I'm making a bit more:

  1. I suspect this encourages more private API usage

Today, if your app or addon uses private API you understand that you are taking on a support risk. APIs become intimate only once enough app's / addon's have taken this risk, but even there it was an acknowledged risk. With this RFC it becomes somewhat easier for addons to abuse private not-previously-intimate APIs with the understanding that by doing so they are forcing support and a major version. This may not be a huge risk, and I think most developers wouldn't be so cruel, but I do think it is a risk since the disincentives of abusing private APIs are lessened.

This may be a risk we happily take on to reduce the risks of "semver drift" still breaking apps that were depending on intimate API. That said, we still have the case of addons like model-fragments, which was presented a public API to replace an intimate API and which refactored instead to use a new private API. E.g. this risk I see is not without precedent.

  1. how this interplays with lockstep may cause release cadence issues for other teams
  2. This RFC should recommend a cadence for deprecation cleanup.

Today deprecations are removed:

  • After a major release (intimate or public)
  • After an LTS release (intimate)

Keep in mind there are multiple core libraries all with their own teams to coordinate and intimate and public API deprecations to handle.

Currently this RFC suggests issuing a major any time a deprecation is removed, but it doesn't specify how long a deprecation should live. Currently deprecations are set to live until the next LTS or next major, but if a major is determined by deprecation removal this policy would result in every release after an LTS release becoming a major release. That would seem to void the value of LTS releases, at least in their current every-4th-release form.

Alternatively a policy could be that deprecations must live for at least-N versions (where N is some number >= 4 in all likelihood). But a policy like that would struggle from every single release becoming a major release, because new deprecations are introduced in most releases.

Formalizing a cadence for deprecation removal would avoid issues with teams forcing majors either unexpectedly or too often on other teams. The cadence should be regular and consistent, and ideally not too long nor too short in between. Too short, we eliminate the value of LTS releases. Too long, the risk of a team missing being ready for a major with an important deprecation effort becomes too high.

@maxbogue
Copy link

Reading through this RFC and PR, it seems to me like "Intimate APIs" are the crux of the problem. The perspective I see in the comments here seems to assume that people are perfectly aware of the difference between public and intimate APIs and are taking on that risk willingly.

I would wager the real danger that this RFC is trying to address is that most people using intimate APIs have absolutely no idea that they are doing so, and therefore are completely blindsided by a minor version bump breaking their application.

I'm not sure what the options are to correct that, but a separate change to clarify what is public and what is private somehow (a naming convention like a prefixed _ in Python?) may be worth considering to address the issue of intimate APIs entirely.

Copy link
Member

rwjblue commented Jul 11, 2019

The line between public and private is fairly clear: anything that is documented must be documented as either @public or @private (enforced by documentation linting), and anything undocumented is private. I totally agree that it may not be clear when hacking around in the debugger, and preferring underscored method names is absolutely a good idea (which I think we try to do?).

@jelhan
Copy link
Contributor

jelhan commented Jul 14, 2019

Currently deprecations are set to live until the next LTS or next major, but if a major is determined by deprecation removal this policy would result in every release after an LTS release becoming a major release. That would seem to void the value of LTS releases, at least in their current every-4th-release form.

@runspired Could you elaborate a little bit more why this would void the value of LTS releases? I don't get that point. Of course such a policy would mean that deprecation of public APIs must life for more than one major release cause they shouldn't be removed that often as intimate APIs but I having a fixed end of life for deprecated public APIs would also make planing easier.

I could see some good arguments for a policy like:

  • Every 4th release bumps the major version and removes deprecated APIs (intimate and public).
  • Every release before a major version is bumped is declared an LTS release.
  • An intimate public API must be deprecated at least one minor release before it's removed.
  • A public API must be deprecated at least four two major releases before it's removed.

This would ensure that every intimate API is deprecated at least six weeks (1 release) and every public API at least ~ 1 ½ years (2 major releases + 1 release = 12 + 1 releases ≈ 78 weeks) before they are removed. Having a date as end of life than a vague v4 would also be more helpful for planing.

@maxbogue
Copy link

@runspired I'm also having trouble following your logic. You seem to be conflating deprecating something (which can be done as often as needed) and removing deprecated things (what this RFC is suggesting only happen at major version bumps). There's no reason it would affect the LTS schedule, it's just that LTS releases wouldn't remove deprecated things anymore.

@runspired
Copy link
Contributor

@jelhan @maxbogue This RFC voids the value of LTS releases because de-facto as-written every release post-current-LTS-cycle becomes a major release.

LTS exists so that there are clearly supported points within the minor release cycle to upgrade between: but if there are only 4 releases between majors then having a specific LTS policy becomes meaningless as there are no minor releases that are LTS releases that aren't simply "the last 1.0/2.0/3.0/4.0" release. Basically, this RFC as-written has the side-effect of replacing the every 4th release being an LTS with every 4th release being a major: resulting in roughly two major versions per year.

This also means that churn goes way up, because with "GC" releases occurring far more frequently the time between deprecation and removal becomes very short.

The biggest downside is that teams would no longer be upgrading from LTS to LTS with the last LTS before a major being a special and more rare occurrence: instead they would be upgrading from major to major.

To be very clear I am not opposed to the ideas this RFC is presenting, but it needs to propose the exact cadence of major releases, and that cadence should not be 1:1 with LTS releases.

@Kerrick
Copy link
Author

Kerrick commented Jul 15, 2019

This also means that churn goes way up, because with "GC" releases occurring far more frequently the time between deprecation and removal becomes very short.

It doesn’t. If a deprecated public API was not intended to be GCed until at least June 2021, nothing says that v4.0 releasing in Feb 2020 due to deprecated private APIs being removed means that public feature has to be GCed. That is based on a flawed premise that ALL deprecations must be GCed on EACH major release. Instead, it can be GCed in v6.0 or whatever the next major number happens to be in June 2021.

@runspired
Copy link
Contributor

@Kerrick every deprecation receives an until major version, they aren't scheduled for a specific date.

@Kerrick
Copy link
Author

Kerrick commented Jul 15, 2019

And as I mentioned in the RFC, the “until” date can be a minimum version, not a maximum. They can stick around past that version.

@runspired
Copy link
Contributor

runspired commented Jul 15, 2019

Also note, if we started doing majors based on length of time a deprecation exists, we get back to the problem that very quickly every release becomes a major version.

Data deprecates something nearly every minor, and we aren't the only library adding deprecations. All libraries adhering to lockstep would be forcing majors.

@runspired
Copy link
Contributor

runspired commented Jul 15, 2019

And as I mentioned in the RFC, the “until” date can be a minimum version, not a maximum. They can stick around past that version.

This policy would introduce confusion, having an implicit agreement that some deprecations will live past their until versions would be difficult to manage and explain. Especially when today the policy already is a minimum version, not a maximum version.

@jelhan
Copy link
Contributor

jelhan commented Jul 15, 2019

@runspired I'm sorry, but I don't get your point.

We could do majors on a fix schedule. I proposed every forth-release. So there wouldn't be any issue that each release could become a major. That would also be similar to the process currently used for intermediate APIs. As today they would be removed every forth-release (one after LTS). The changes to the current process would be: a) the version removing intermediate APIs is a major one; b) there is a fixed date for removing public API in opposite to an unscheduled v4.

An API could be deprecated nth-major releases in advance. E.g. if we go with four-major releases in advance for public APIs, a deprecation of a public API introduced in v7.2 would have an until of v11. If we notice in the process that the ecosystem isn't catching up quick enough, the deprecation's until might be changed in v9.0 to v13. I don't think that would introduce any teaching issues.

We would still be able to garbage collect deprecated intermediate APIs quickly if we decide that they could be removed in next major. That would mean that an intermediate API introduced in v7.3 will be removed in v8.

Having garbage collection even for public APIs on a regular schedule would fit perfectly to the stability without stagnation concept.

@runspired
Copy link
Contributor

runspired commented Jul 15, 2019

@jelhan it seems you do get my point. A regular cadence of majors is necessary for lockstep to work, for teams to be able to GC, and for users to have clear expectations of when things will be removed. Whether that is every 4th or 8th or something else.

The point I keep re-emphasizing is that unless this RFC proposes such a cadence, it leads to either a situation in which every release is a major (obviously undesirable) or in which every 4th is a major (making LTS fairly unimportant other than to demarcate majors) with larger public-API GCs happening more frequently (if that's the intent it should be clear).

What introduces teaching issues is if we use until to implicitly mean different things as was suggested by others. (e.g. a public API "until 4.0" really means until "5.0 or 6.0" where a private API really does mean "until 4.0", that sort of confusion).

My personal recommendation is that "if" we were to keep lockstep then we should do a major release every 8th release. There is a much more important discussion that this RFC does not acknowledge but which needs to be had and consensus reached upon as to whether lockstep is still worth it and still viable, as it has led to a lot of semver issues (including some of the "breaking" LTS cleanups) and made it difficult for individual teams to plan and execute the removal of deprecations in a timely way.

e.g. a pattern like the following which would roughly equate to two LTS releases and one major release per year.

3.12 LTS
4.0 -> 4.4 LTS -> 4.8 LTS
5.0 -> 5.4 LTS -> 5.8 LTS
6.0

@Kerrick
Copy link
Author

Kerrick commented Jul 16, 2019

I had never considered that lockstep releases could be the root cause of the issues that caused me to write this RFC. That’s an interesting perspective...

@chriskrycho
Copy link
Contributor

My own take is twofold:

  1. "Intimate API" was a bad idea. I know why we did it, but sometimes it's possible to over-do trying to help people. When you choose to use private API, that's on you! And it's okay; sometimes it's the right call. But then it's debt to be paid, and you have to understand the tradeoffs you make when you're making that move. That's part of the job!

  2. Kill lockstep. We need a new way of keeping track of how things stay in sync, though, to make that work – perhaps Ember CLI Update becomes a core primitive of the ecosystem, and/or packages have to advertise the ranges they support. This is more honest in some ways—per @runspired's comments about effectively-breaking changes within LTS releases, esp. in Ember Data—but it's also harder. There's a lot of design work to be done there. I think it's the right thing, and likely necessary, but it's hard.

@wycats wycats self-assigned this Jul 16, 2019
@Gaurav0
Copy link
Contributor

Gaurav0 commented Jul 19, 2019

People have already said most of what I'd like to say, so I'll just generally agree with @chriskrycho and @runspired

I'd like to add the following suggestions:

  1. Release 4.0 soonish
  2. When deprecating an intimate API, as part of the deprecation process, search for add-ons that use it and publish a list, perhaps in the PR.
  3. Announce upcoming intimate api deprecations in places like Ember Times proactively.
  4. Make widely used apis public.
  5. If there are any frowned upon public apis that might be deprecated soon, find a way to let people know.

@MelSumner
Copy link
Contributor

@chriskrycho I reconciled with intimate API as a necessary thing when you're trying to create something that people use IRL- kind of like the architect who waited to install sidewalks on a campus until he saw how people decided to get from place to place. I wouldn't want it advertised in official documentation, because then it might as well not exist; but I do think it should be found when driven by genuine need to do something, as it can point out the gaps we have for making useful things (since no one can be expected to know how to make all the things).

@Kerrick
Copy link
Author

Kerrick commented Sep 9, 2019

What's the best way to move forward with the ideas presented in this RFC and/or the discussion on this PR?

@acorncom
Copy link
Contributor

@Kerrick Per the discussion of "The Process", it looks like we're probably at the "find a champion on the relevant core team" stage here ...

@lupestro
Copy link
Contributor

If a private API is used widely enough for deprecation to be deemed wise, that is an acknowledgment that it has historically met a broad need that isn't addressed by the public APIs. If so, we need to complete that acknowledgement by declaring it public in the same release in which it is deprecated. Then:

  1. we take on the same design difficulties we have supporting a public API that needs to continue to be supported as the world changes around it.
  2. we don't have to do anything new to our cadence except hang onto supporting the old behavior until a major release.

Making more frequent major releases will make our problems worse:

  1. We no longer have the confidence that if we see "deprecated" in our development runs, unless we schedule starting work on it now, we're going to be in major pain when the {n+1}.0.0 comes out. That's a tremendous incentive for us.
  2. Dealing with even a single deprecation of a practice that we routinely were doing - especially if we weren't aware that we were doing it - can sometimes require peeling off one team member for several months of work on the day the deprecation first appears and we need all the lead time we can get to address it.
  3. The whole point of LTS releases is that teams can move their code from LTS to LTS without dealing with the churn of intermediate releases. Hence, we will first see a deprecation triggered in our own code on the first LTS after that deprecation first appears.

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

There is active movement around #830 which may address the issues raised here. I'll leave it open for now but it's possible that it will be superseded.

@chriskrycho chriskrycho assigned chriskrycho and unassigned wycats Jul 29, 2022
@chriskrycho chriskrycho added FCP to close The core-team that owns this RFC has moved that this RFC be closed. T-ember-data RFCs that impact the ember-data library T-framework RFCs that impact the ember.js library T-learning T-ember-cli RFCs that impact the ember-cli library labels Jul 29, 2022
@chriskrycho
Copy link
Contributor

We discussed this RFC at today’s Framework Core meeting and decided to move it into Final Comment Period to Close. (FCP to close means that we will close this next week unless there are substantive new points brought up in discussion which are not already represented in the discussion above.) We believe RFC #830 addresses the underlying issues this RFC was pointing at, and does so by addressing them at a more fundamental level in our process, by providing a more regular cadence for dealing with not only private and intimate API but also public API.


Two really important notes we all agreed we wanted to call out here:

  1. This RFC was really helpful in “priming the pump” for the direction we are now heading with RFC: Evolving Ember's Major Version Process #830, and as @jelhan pointed out over there, this discussion was one of the first places where the idea of a more regular cadence come up. Thanks very much to @Kerrick for taking the time to write up the idea and for kicking off the discussion here. Anyone who has written an RFC knows that it is a lot of work!

  2. In general, we do not see closing an RFC like this as a sign that the RFC was bad, misguided, wrong, etc. Instead, it often means the exact opposite: that a given RFC started a conversation, or got things moving in the right direction, or provided useful background and discussion along the way.

@ef4 ef4 closed this Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FCP to close The core-team that owns this RFC has moved that this RFC be closed. T-ember-cli RFCs that impact the ember-cli library T-ember-data RFCs that impact the ember-data library T-framework RFCs that impact the ember.js library T-learning
Projects
None yet
Development

Successfully merging this pull request may close these issues.