-
Notifications
You must be signed in to change notification settings - Fork 393
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
docs: Document the existing deprecation policy #5545
base: main
Are you sure you want to change the base?
Conversation
We recently renamed `jj util mangen` to `jj install-manpages` without a deprecation warning, as it was missed in the initial PR. Waleed then fixed it in a follow-up, which led to a longer discussion in the Discord which revealed that not everyone was aware of it and _what_ requires a warning based deprecation. This initially categorizes the policy into user-visible UX changes and small impact renames, like for a small usergroup like packagers and third-party dependencies. If we have more hickups with users, this can be expanded.
b6f3c3b
to
03f4412
Compare
## Deprecating and removing commands | ||
|
||
Before removing a until now required argument, or making a optional argument | ||
required or even a user-visible command, make yourself familiar with our |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"or even a user-visible command" seems out of place.
|
||
Whenever you rename a command or make a previously optional argument required, | ||
we require that you preserve the old command invocations keeps on working for 6 | ||
months (so 6 releases, since we release monthly) with a deprecation message. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(For discussion) This might be a mostly a stylistic difference, but I'd prefer to make this document softer.
For example, I'd say,
When we rename commands or make previously optional arguments required, we usually try to keep the old command invocations working for 6 months...
(there are also a few unrelated fixups in the above)
We could also insert
When we rename commonly used commands...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, IMO, these are more like rules of thumb than something we should strictly enforce. I think we should always balance how hard it is to keep compatibility vs how hard it is for users to adapt. I mentioned on Discord the example of file-level vs tree-level conflicts. In that case, dropping compatibility completely means that users have to re-clone their repo. As a result, we have been much more conservative with that deprecation. We still support file-level conflicts in old repos over a year later.
I think (but I may be wrong) when we originally started talking about the 6-month policy, it was about storage formats. I think it's fair to have a strict rule in that case saying that a repo created with a jj version less than 6 months old should be readable by the latest version.
For commands with a niche user audience or something we assume is rarely used | ||
(we sadly have no data), we take the liberity to remove the old behavior within | ||
two releases. This means that you can change the old command to immediately | ||
error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean about "within two releases"? This seems to contradict making the command to immediately error.
The message should inform the user that the previous workflow is deprecated | ||
and to be removed in the future. | ||
|
||
## Packaging commands and niche commands |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(For discussion) You don't really discuss packaging commands in here. I think it'd make sense to have a separate section about how we treat packager-facing deprecations, but I'm not sure what it should say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @emilazy since she brought it up in the Discord thread. I'm also happy to combine it with the next section if that works for you.
|
||
## Third-Party dependencies | ||
|
||
For third-party dependencies which previously were used for a core functionality |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(For discussion) This section doesn't make a lot of sense to me. Perhaps it's OK to just remove it, in favor of a possible future "packager-facing deprecations" section.
Before removing a until now required argument, or making a optional argument | ||
required or even a user-visible command, make yourself familiar with our | ||
[Deprecation policy](deprecation.md). This makes the tool pleasant to use even | ||
if the hasn't reached a 1.0 yet. It allows changes like the `branch` to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: "if the hasn't"
@@ -82,6 +82,14 @@ existing component, we require an architecture review from multiple | |||
stakeholders, which we do with [Design Docs](design_docs.md), see the | |||
[process here](design_docs.md#process). | |||
|
|||
## Deprecating and removing commands | |||
|
|||
Before removing a until now required argument, or making a optional argument |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the "until now" just makes it harder to read. I think it's obvious that it won't be require once it's removed. Hyphenate and change the article to "an" if you really want to keep it: "an until-now" (but I'm not a native speaker)
[Deprecation policy](deprecation.md). This makes the tool pleasant to use even | ||
if the hasn't reached a 1.0 yet. It allows changes like the `branch` to | ||
`bookmark` rename or the removal of `checkout` and `merge` to happen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was confusing to me. I think it's because it talks about both what the policy prevents and what it allows without a clear separation. Is the bit about "makes the tool pleasant to use" about not breaking things? Or about enabling us to make progress on the UX?
Maybe something like the following instead?
The deprecation policy is designed to allow us to make progress quickly without breaking workflows for users too frequently.
## Packaging commands and niche commands | ||
|
||
For commands with a niche user audience or something we assume is rarely used | ||
(we sadly have no data), we take the liberity to remove the old behavior within |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo in "liberity"
two releases. This means that you can change the old command to immediately | ||
error. | ||
|
||
## Third-Party dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: lowercase "party" for consistency
@@ -0,0 +1,23 @@ | |||
# Deprecation Policy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels natural to talk about breaking changes together with deprecations. I don't want to expand the scope too much. I'm just mentioning this in case you think it should affect the name of the file or the heading. I personally think it's fine to cover breaking changes without changing either.
For commands with a niche user audience or something we assume is rarely used | ||
(we sadly have no data), we take the liberity to remove the old behavior within | ||
two releases. This means that you can change the old command to immediately | ||
error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can mention jj debug
commands specifically. I think breaking changes in them is always fine. But we can still try to balance it against how hard it is to keep compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can mention
jj debug
commands specifically. I think breaking changes in them is always fine. But we can still try to balance it against how hard it is to keep compatibility.
While I feel like most of the jj debug
commands behave like an internal API and shouldn't get advertised even more widely, they are a great example.
|
||
Whenever you rename a command or make a previously optional argument required, | ||
we require that you preserve the old command invocations keeps on working for 6 | ||
months (so 6 releases, since we release monthly) with a deprecation message. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, IMO, these are more like rules of thumb than something we should strictly enforce. I think we should always balance how hard it is to keep compatibility vs how hard it is for users to adapt. I mentioned on Discord the example of file-level vs tree-level conflicts. In that case, dropping compatibility completely means that users have to re-clone their repo. As a result, we have been much more conservative with that deprecation. We still support file-level conflicts in old repos over a year later.
I think (but I may be wrong) when we originally started talking about the 6-month policy, it was about storage formats. I think it's fair to have a strict rule in that case saying that a repo created with a jj version less than 6 months old should be readable by the latest version.
We recently renamed
jj util mangen
tojj install-manpages
without a deprecation warning, as it was missed in the initial PR. Waleed then fixed it in a follow-up, which led to a longer discussion in the Discord which revealed that not everyone was aware of it and what requires a warning based deprecation.This initially categorizes the policy into user-visible UX changes and small impact renames, like for a small usergroup like packagers and third-party dependencies. If we have more hickups with users, this can be expanded.