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

RFD: Soften deprecation of jj checkout/merge #3218

Closed
wants to merge 4 commits into from

Conversation

martinvonz
Copy link
Member

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

@matts1 matts1 self-requested a review March 5, 2024 05:58
@martinvonz
Copy link
Member Author

@thoughtpolice: do you want a look at this before I merge it?

Copy link
Collaborator

@PhilipMetzger PhilipMetzger left a comment

Choose a reason for hiding this comment

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

I really dislike reverting the stance on this, as it will just set expectations which we'll need to uphold. But I don't think my opinion on this will impact the decision.

I think users will still slowly get used to running jj new instead of
jj checkout. I think this patch might just make that process slower
(bad) and less frustrating (good).

I don't think that this will help people understand the new model. If we just paper over it, people will always expect that these commands work.

cli/src/commands/checkout.rs Show resolved Hide resolved
I've heard several people say that deleting `jj checkout` is too
extreme. We don't want to scare away potential new users. There's very
little cost involved in keeping the command around indefinitely. So I
think we should at least remove the promise to delete it in the
future.
This way users can override `jj co` to mean `jj new` if they want to
get rid of the warning.
It can take a while to get used to the slightly different mental model
of `jj new` if you're used to `git/hg checkout`. Let's help users
define an alias to avoid the warning `jj checkout` currently prints. I
think users will still slowly get used to running `jj new` instead of
`jj checkout`. I think this patch might just make that process slower
(bad) and less frustrating (good).
@martinvonz
Copy link
Member Author

I don't think that this will help people understand the new model. If we just paper over it, people will always expect that these commands work.

That's honestly fine with me. Some people are more change-averse than others. If this can help those people switch to jj, that's a positive IMO, even if they stick to jj co for five years after switching.

@PhilipMetzger
Copy link
Collaborator

I don't think that this will help people understand the new model. If we just paper over it, people will always expect that these commands work.

That's honestly fine with me. Some people are more change-averse than others. If this can help those people switch to jj, that's a positive IMO

Agreed, so providing them with a alias is fine. But I'd really like to see that command gone before a 1.0 (which won't happen, as it breaks to many users), as it doesn't make sense in the jj model.

@thoughtpolice
Copy link
Collaborator

thoughtpolice commented Mar 5, 2024

What was the motivation for this? I suppose I missed it.

I'm fine with removing the hard deadline dates. That said, I'd still like to get rid of the code in the long run and I don't think we should encourage people to use these unless they've already been using them. I guess that's my full opinion.

@PhilipMetzger
Copy link
Collaborator

PhilipMetzger commented Mar 5, 2024

See the discussion in #2842, begins here #2842 (comment)

@martinvonz
Copy link
Member Author

What was the motivation for this? I suppose I missed it.

It's just what I mentioned in the commit messages, i.e. that the "will be deleted text" can be off-putting to new users.

I'm reasonably fine with removing the hard deadline dates. That said, I'd still like to get rid of the code in the long run and I don't think we should encourage people to use these unless they've already been using them. I guess that's my full opinion.

I agree, but "in the long run" is probably at least 5 years IMO. I see very little reason to remove it until something like 95% of users have voluntarily switched over to using jj new.

By the way, it would be nice to come up with a verb for what jj new does (create a commit and point the working copy to it). That's something I miss with jj new compared to git/hg/jj checkout.

@thoughtpolice
Copy link
Collaborator

FWIW, the only reason I chose "later this year" was mostly so that we wouldn't forget about it forever. The code debt probably isn't too bad since they can both be implemented in terms of new. But we can probably also have an annual cleaning spree once every year or something. So we can just see how we feel about it later down the road, probably.

@joyously
Copy link

joyously commented Mar 5, 2024

By the way, it would be nice to come up with a verb for what jj new does

start? beget? hatch? birth?
work? hack?
close? or just keep commit?

@martinvonz martinvonz changed the title Soften deprecation of jj checkout/merge RFD: Soften deprecation of jj checkout/merge Mar 5, 2024
@martinvonz
Copy link
Member Author

martinvonz commented Mar 5, 2024

I've heard that some people think this PR was an expression of me pushing Google's agenda. I really don't think the concern about jj checkout being deleted is specific to Google, and I didn't even consider that this was in Google's interest. I've just heard the concern from several people (who all happen to be Googlers, I admit) and wanted to make sure we reduce the risk of scaring away new users in the future. Sorry that I didn't realize that my role on the project might make it feel like this PR had to be approved. I can drop the PR if that's the project's consensus, but I'm a bit worried that that means we will scare away some new users.

Copy link
Collaborator

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

Is there a way to have the message suggest the replacement? Because to me, the hardness of it is that it seems like it's going away with no suggested alternative.

@martinvonz
Copy link
Member Author

Is there a way to have the message suggest the replacement? Because to me, the hardness of it is that it seems like it's going away with no suggested alternative.

The line before the message says this:

warning: jj checkout is deprecated; use jj new instead, which is equivalent

So I think we're good from that perspective. My concern is not giving existing users a smooth transition. My concern is that future users (after we delete jj checkout) who have never use jj before will run jj checkout and get an error. Even if that error clearly says that they should use jj new, I'm worried that the error be jarring enough to scare some of them away. I may be wrong, of course.

@thoughtpolice
Copy link
Collaborator

I've heard that some people think this PR was an expression of me pushing Google's agenda. I really don't think the concern about jj checkout being deleted is specific to Google, and I didn't even consider that this was in Google's interest. I've just heard the concern from several people (who all happen to be Googlers, I admit) and wanted to make sure we reduce the risk of scaring away new users in the future. Sorry that I didn't realize that my role on the project might make it feel like this PR had to be approved. I can drop the PR if that's the project's consensus, but I'm a bit worried that that means we will scare away some new users.

I posted a response to this on Discord just to summarize my thoughts on the matter. Copy/pasting for clarity here:


  • I don't consider everything from you final. I think you've done a fantastic job leading the project, I've never gotten that impression. Frankly, I think this is probably a case where nobody wants to seem too "mad" or "harsh" over it. After all, small battles are the easiest to fight. It's not a big deal. I'd rather save my energy pushing back on your changes for real things I disagree with. :)
  • I have said this before, but: I do not consider Google or Google's needs inside of my mind palace. I'm glad that other Googlers are aware of this and brought it up, though. It shows that the project is clearly perceived as "not a google thing", and that's an image we should aim to keep and sustain.
  • Occasionally, it might crop up e.g. #3034 was a bit annoying. I don't really care, though. Actually, that's probably a better example of the behavior you might be worried about being perceived as "Google controlled". :) Personally, I just accepted it as a fact of life. It's unbelievably small. Whatever. Presumably there is some ancient dilapidated Google Thing behind closed doors that checks copyright headers, and nobody has updated it in a few years to understand SPDX. Maybe it's just a rule from Lawyers(TM). Don't know, don't care, maybe it will be solved one day later as the project blossoms more and we move to an org, stuff like that.
  • Rest assured that if I feel I have issues with the direction of things being "Googlified" in some way that isn't healthy for the project, I will make my own concerns apparent and clear. A bit of balancing on both sides is needed.
  • Roll with the punches. I think everything is going great and the extra changes from internal Googlers is welcome.

Again, small battles are the easiest to fight. I've been the Really Grumpy OSS Guy before. It's not a good and healthy way to be. So I avoid that. Save it for things that truly matter, and trust the people you're working with, and the hard parts will pass in time. Them's my rules, and I'm sticking to 'em these days.

(And I do appreciate that Googlers are receptive to the way the project is perceived. It's important to know what image of yourself you want to project.)

@necauqua
Copy link
Collaborator

necauqua commented Mar 5, 2024

my two cents:
In my ideal world, in 1.0+ those commands do not exist, but there are hints for people migrating from git.
I'm ok with actual aliases being there forever if that's what the final decision is, but I fully agree that these should not be real subcommands.

I'm worried that the error be jarring enough to scare some of them away. I may be wrong, of course.

I think if there's an encouraging happy message that's not an error, but a hint, "hi, to do this you have to use jj new, it makes so much sense, you'll see" then it should be totally fine 🙃

@matts1
Copy link
Collaborator

matts1 commented Mar 6, 2024

@PhilipMetzger I'm getting a bit of mixed signals from your comments, so just wanted to clarify your stance

My main point is being unique with not having these commands at all. The best moment to remove something is yesterday, today or not at all.

Agreed, so providing them with a alias is fine. But I'd really like to see that command gone before a 1.0

Reading between the lines, if I understand correctly, you're saying you'd be fine if we replaced the command with an alias that was included in jj by default, and that your problem is the maintainence burden of checkout, and so you'd like to get the actual command removed?

If that indeed is your perspective, I think I agree with you, but at the same time, that's not a user-facing change. Thus, from a user's perspective, having a warning saying "hey, this command will be removed" is incorrect, since when it's removed it'll just be replaced with an alias and still do the exact same thing. To be clear, I'm definitely not advocating that we don't remove the actual command, I'm just advocating that a user coming from git should be able to say jj checkout.

@steveklabnik
Copy link
Collaborator

The line before the message says this:

warning: jj checkout is deprecated; use jj new instead, which is equivalent

So I think we're good from that perspective.

don't know how i missed this! yeah, sounds good

@martinvonz
Copy link
Member Author

It sounds like the consensus is to keep the current behavior, so I'll drop this PR. I'll move the last commit (avoiding jj co in tests) to a separate PR.

@martinvonz martinvonz closed this Mar 6, 2024
@martinvonz martinvonz deleted the push-pptormqmoqqn branch March 6, 2024 14:36
@PhilipMetzger
Copy link
Collaborator

I initially posted this on Discord but for full public disclosure it also belongs on to this thread. Although I took the liberity to make some edits to be hopefully less hurtful.

I apologize to everyone involved for the fuss I made.
Austin is probably better with words, but I'll try my best.

  • I like how jj is managed, be it with some Google involvement or not. I don't think Googles involvement is harmful at all.
  • Everyone in here deserves an audience and to be treated equally, so it is unexpected to see such an issue pop up so late, as there was plenty of time to discuss it.
  • I also really care about the image of the project and a hypothetical stunt like a one day turnaround without community involvement can have long-term repercussions.

Yesterday I really was frustrated on how it happened, as the concerns never reached OSS users when other communication went quite smoothly.
I'm happy to to take the blame, as my name is attached everywhere.

Addendum:
I have now realized that I am the bad actor and have caused way more trouble than it was worth. This a large failure on my side, as I didn't give Martin and Matt the benefit of the doubt.

@yuja
Copy link
Collaborator

yuja commented Mar 7, 2024

cli: move jj co alias to config file, so it can be overridden

@martinvonz I think this patch is nice and non-controversial. Could you send as a separate PR?

@martinvonz
Copy link
Member Author

cli: move jj co alias to config file, so it can be overridden

@martinvonz I think this patch is nice and non-controversial. Could you send as a separate PR?

Sent as #3242

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants