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

refactor: remove 'jj checkout' command #1713

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

Copy link
Contributor

Choose a reason for hiding this comment

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

Re the commit's and PR title: I wouldn't call this a "refactor". I'd put "cli" instead (not sure if other people have better thoughts).

### Breaking changes

* The command `jj checkout` (shorthand `jj co`) has been removed; the hope is
to avoid confusion when compared to `git checkout`, which has subtly different
semantics. Use `jj new` instead, which can do everything `jj co` could, and it
can also perform merges as well.

* The `ui.oplog-relative-timestamps` option has been removed. Use the
`format_time_range()` template alias instead. For details, see
[the documentation](docs/config.md).
Expand Down
91 changes: 42 additions & 49 deletions docs/contributing.md
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible, please limit the reformatting to the paragraphs you actually changed. If you'd like to reformat the whole file, that can be a separate commit.

In this case, I'm not sure the reformatting is for the better, but I only glanced at it; feel free to have a reformatting commit if you feel it is for the better.

If this is difficult, we can leave it be.

Copy link
Member Author

Choose a reason for hiding this comment

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

Damn, I thought I avoided this but apparently not. It was an accident in vscode and I guess my changes got slurped up. I'll back this out and fix it.

Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# How to Contribute


## Policies

We'd love to accept your patches and contributions to this project. There are
Expand Down Expand Up @@ -41,21 +40,19 @@ message.

When you address comments on a PR, don't make the changes in a commit on top (as
is typical on GitHub). Instead, please make the changes in the appropriate
commit. You can do that by checking out the commit (`jj checkout/new <commit>`)
and then squash in the changes when you're done (`jj squash`). `jj git push`
will automatically force-push the branch.
commit. You can do that by checking out the commit (`jj new <commit>`) and then
squash in the changes when you're done (`jj squash`). `jj git push` will
automatically force-push the branch.

When your first PR has been approved, we typically give you contributor access,
so you can address any remaining minor comments and then merge the PR yourself
when you're ready. If you realize that some comments require non-trivial
changes, please ask your reviewer to take another look.


### Community Guidelines

This project follows [Google's Open Source Community
Guidelines](https://opensource.google/conduct/).

This project follows
[Google's Open Source Community Guidelines](https://opensource.google/conduct/).

## Learning Rust

Expand All @@ -64,14 +61,12 @@ excellent resources at https://www.rust-lang.org/learn, we recommend the
["Comprehensive Rust" mini-course](https://google.github.io/comprehensive-rust/)
for an overview, especially if you are familiar with C++.


## Setting up a development environment

To develop `jj`, the mandatory steps are simply
to [install Rust](https://www.rust-lang.org/tools/install) (the default
installer options are fine), clone the repository, and use `cargo build`
, `cargo fmt`,
`cargo clippy --workspace --all-targets`, and
To develop `jj`, the mandatory steps are simply to
[install Rust](https://www.rust-lang.org/tools/install) (the default installer
options are fine), clone the repository, and use `cargo build` , `cargo fmt`,
`cargo clippy --workspace --all-targets`, and\
`cargo test --workspace`. If you are preparing a PR, there are some additional
recommended steps.

Expand All @@ -95,60 +90,58 @@ During development (adapt according to your preference):
cargo insta test --workspace # Occasionally

WARNING: Build artifacts from debug builds and especially from repeated
invocations of `cargo test` can quickly take up 10s of GB of disk space.
Cargo will happily use up your entire hard drive. If this happens, run
`cargo clean`.
invocations of `cargo test` can quickly take up 10s of GB of disk space. Cargo
will happily use up your entire hard drive. If this happens, run `cargo clean`.

### Explanation

These are listed roughly in order of decreasing importance.

1. Nearly any change to `jj`'s CLI will require writing or updating snapshot
tests that use the [`insta`](https://insta.rs/) crate. To make this
convenient, install the `cargo-insta` binary.
Use `cargo insta test --workspace` to run tests,
and `cargo insta review --workspace` to update the snapshot tests.
The `--workspace` flag is needed to run the tests on all crates; by default,
only the crate in the current directory is tested.
convenient, install the `cargo-insta` binary. Use
`cargo insta test --workspace` to run tests, and
`cargo insta review --workspace` to update the snapshot tests. The
`--workspace` flag is needed to run the tests on all crates; by default, only
the crate in the current directory is tested.

2. Github CI checks require that the code is formatted with the *nightly*
2. Github CI checks require that the code is formatted with the _nightly_
version of `rustfmt`. To do this on your computer, install the nightly
toolchain and use `cargo +nightly fmt`.

3. Your code will be rejected if it cannot be compiled with the minimal
supported version of Rust ("MSRV"). This version is listed as
`rust-version` in [`Cargo.toml`](../Cargo.toml); it is 1.64 as of this
writing.
supported version of Rust ("MSRV"). This version is listed as `rust-version`
in [`Cargo.toml`](../Cargo.toml); it is 1.64 as of this writing.

4. Your code needs to pass `cargo clippy`. You can also
use `cargo +nightly clippy` if you wish to see more warnings.
4. Your code needs to pass `cargo clippy`. You can also use
`cargo +nightly clippy` if you wish to see more warnings.

5. You may also want to install and use `cargo-watch`. In this case, you should
exclude `.jj`. directory from the filesystem watcher, as it gets updated on
every `jj log`.

6. To run tests more quickly, use `cargo nextest run --workspace`. To
use `nextest` with `insta`,
use `cargo insta test --workspace --test-runner nextest`.
## Modifying protobuffers (this is not common)
Occasionally, you may need to change the `.proto` files that define jj's data
storage format. In this case, you will need to add a few steps to the above
workflow.
- Install the `protoc` compiler. This usually means either `apt-get install
protobuf-compiler` or downloading [an official release]. The
[`prost` library docs] have additional advice.
- Run `cargo run -p gen-protos` regularly (or after every edit to a `.proto`
file). This is the same as running `cargo run` from `lib/gen-protos`. The
`gen-protos` binary will use the `prost-build` library to compile the
`.proto` files into `.rs` files.
- If you are adding a new `.proto` file, you will need to edit the list of
these files in `lib/gen-protos/src/main.rs`.
6. To run tests more quickly, use `cargo nextest run --workspace`. To use
`nextest` with `insta`, use
`cargo insta test --workspace --test-runner nextest`.

## Modifying protobuffers (this is not common)

Occasionally, you may need to change the `.proto` files that define jj's data
storage format. In this case, you will need to add a few steps to the above
workflow.

- Install the `protoc` compiler. This usually means either
`apt-get install protobuf-compiler` or downloading [an official release]. The
[`prost` library docs] have additional advice.
- Run `cargo run -p gen-protos` regularly (or after every edit to a `.proto`
file). This is the same as running `cargo run` from `lib/gen-protos`. The
`gen-protos` binary will use the `prost-build` library to compile the `.proto`
files into `.rs` files.
- If you are adding a new `.proto` file, you will need to edit the list of these
files in `lib/gen-protos/src/main.rs`.

[an official release]: https://github.com/protocolbuffers/protobuf/releases
[`prost` library docs]: https://docs.rs/prost-build/latest/prost_build/#sourcing-protoc

The `.rs` files generated from `.proto` files are included in the repository,
and there is a Github CI check that will complain if they do not match.
The `.rs` files generated from `.proto` files are included in the repository,
and there is a Github CI check that will complain if they do not match.
2 changes: 1 addition & 1 deletion docs/technical/concurrency.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ Because we allow branches (etc.) to be in a conflicted state rather than just
erroring out when there are multiple heads, the user can continue to use the
repo, including performing further operations on the repo. Of course, some
commands will fail when using a conflicted branch. For example,
`jj checkout main` when `main` is in a conflicted state will result in an error
`jj new main` when `main` is in a conflicted state will result in an error
telling you that `main` resolved to multiple revisions.

### Storage
Expand Down
6 changes: 2 additions & 4 deletions docs/tutorial.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,7 @@ stayed the same. Each change to the working-copy commit amends the previous
version. So how do we tell Jujutsu that we are done amending the current change
and want to start working on a new one? That is what `jj new` is for. That will
create a new commit on top of your current working-copy commit. The new commit
is for the working-copy changes. For familiarity for user coming from other
VCSs, there is also a `jj checkout/co` command, which is practically a synonym
for `jj new` (you can specify a destination for `jj new` as well).
is for the working-copy changes.

So, let's say we're now done with this change, so we create a new change:
```shell script
Expand All @@ -106,7 +104,7 @@ Alternatively, we can use `jj edit <commit>` to resume editing a commit in the
working copy. Any further changes in the working copy will then amend the
commit. Whether you choose to checkout-and-squash or to edit typically depends
on how done you are with the change; if the change is almost done, it makes
sense to use `jj checkout` so you can easily review your adjustments with
sense to use `jj new` so you can easily review your adjustments with
`jj diff` before running `jj squash`.

## The log command and "revsets"
Expand Down
42 changes: 0 additions & 42 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ enum Commands {
Branch(branch::BranchSubcommand),
#[command(alias = "print")]
Cat(CatArgs),
Checkout(CheckoutArgs),
Commit(CommitArgs),
#[command(subcommand)]
Config(ConfigSubcommand),
Expand Down Expand Up @@ -226,23 +225,6 @@ struct ConfigEditArgs {
pub config_args: ConfigArgs,
}

/// Create a new, empty change and edit it in the working copy
///
/// For more information, see
/// https://github.com/martinvonz/jj/blob/main/docs/working-copy.md.
#[derive(clap::Args, Clone, Debug)]
#[command(visible_aliases = &["co"])]
struct CheckoutArgs {
/// The revision to update to
revision: RevisionArg,
/// Ignored (but lets you pass `-r` for consistency with other commands)
#[arg(short = 'r', hide = true)]
unused_revision: bool,
/// The change description to use
#[arg(long, short, default_value = "")]
message: DescriptionArg,
}

/// Stop tracking specified paths in the working copy
#[derive(clap::Args, Clone, Debug)]
struct UntrackArgs {
Expand Down Expand Up @@ -1173,29 +1155,6 @@ fn cmd_config_edit(
run_ui_editor(command.settings(), &config_path)
}

fn cmd_checkout(
ui: &mut Ui,
command: &CommandHelper,
args: &CheckoutArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let target = workspace_command.resolve_single_rev(&args.revision)?;
let mut tx =
workspace_command.start_transaction(&format!("check out commit {}", target.id().hex()));
let commit_builder = tx
.mut_repo()
.new_commit(
command.settings(),
vec![target.id().clone()],
target.tree_id().clone(),
)
.set_description(&args.message);
let new_commit = commit_builder.write()?;
tx.edit(&new_commit).unwrap();
tx.finish(ui)?;
Ok(())
}

fn cmd_untrack(
ui: &mut Ui,
command: &CommandHelper,
Expand Down Expand Up @@ -3371,7 +3330,6 @@ pub fn run_command(ui: &mut Ui, command_helper: &CommandHelper) -> Result<(), Co
Commands::Version(sub_args) => cmd_version(ui, command_helper, sub_args),
Commands::Init(sub_args) => cmd_init(ui, command_helper, sub_args),
Commands::Config(sub_args) => cmd_config(ui, command_helper, sub_args),
Commands::Checkout(sub_args) => cmd_checkout(ui, command_helper, sub_args),
Commands::Untrack(sub_args) => cmd_untrack(ui, command_helper, sub_args),
Commands::Files(sub_args) => cmd_files(ui, command_helper, sub_args),
Commands::Cat(sub_args) => cmd_cat(ui, command_helper, sub_args),
Expand Down
106 changes: 0 additions & 106 deletions tests/test_checkout.rs

This file was deleted.

2 changes: 1 addition & 1 deletion tests/test_diffedit_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fn test_diffedit_merge() {
test_env.jj_cmd_success(&repo_path, &["branch", "create", "b"]);
std::fs::write(repo_path.join("file1"), "b\n").unwrap();
std::fs::write(repo_path.join("file2"), "b\n").unwrap();
test_env.jj_cmd_success(&repo_path, &["co", "@-"]);
test_env.jj_cmd_success(&repo_path, &["new", "@-"]);
test_env.jj_cmd_success(&repo_path, &["new"]);
std::fs::write(repo_path.join("file1"), "c\n").unwrap();
std::fs::write(repo_path.join("file2"), "c\n").unwrap();
Expand Down
Loading