-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
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 commentThe reason will be displayed to describe this comment to others. Learn more. "or even a user-visible command" seems out of place. |
||
[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 commentThe reason will be displayed to describe this comment to others. Learn more. typo: "if the hasn't" |
||
`bookmark` rename or the removal of `checkout` and `merge` to happen. | ||
Comment on lines
+89
to
+91
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
|
||
|
||
## Contributing to the documentation | ||
|
||
We appreciate [bug | ||
|
@@ -110,7 +118,6 @@ directory](https://github.com/jj-vcs/jj/tree/main/cli/src/commands). Working | |
on them requires setting up a Rust development environment, as described | ||
below, and may occasionally require adjusting a test. | ||
|
||
|
||
## Learning Rust | ||
|
||
In addition to the [Rust Book](https://doc.rust-lang.org/book/) and the other | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
# Deprecation Policy | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
||
## User-facing commands and their arguments | ||
|
||
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 commentThe 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,
(there are also a few unrelated fixups in the above) We could also insert
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
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 commentThe 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 commentThe 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. |
||
|
||
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 commentThe 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Comment on lines
+13
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can mention There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
While I feel like most of the |
||
|
||
## Third-Party dependencies | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: lowercase "party" for consistency |
||
|
||
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 commentThe 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. |
||
like `libgit2` was before the `[git.subprocess]` option was added, we're free | ||
to remove most codepaths and move it to a `cargo` feature which we support | ||
up to 6 releases, this is to ease transition for package managers. |
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)