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

commit: add --reset-author option #3998

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

scott2000
Copy link
Collaborator

Since jj describe --reset-author works, I also expected jj commit --reset-author to work, so I think this would be a good option to add.

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

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.

LG, it also makes sense if you read commit as desc + new.

Copy link
Collaborator

@yuja yuja left a comment

Choose a reason for hiding this comment

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

It might be okay for jj commit to always reset author (or timestamp at least.) Is that surprising?

@scott2000
Copy link
Collaborator Author

It might be okay for jj commit to always reset author (or timestamp at least.) Is that surprising?

I think this might be surprising, since as @PhilipMetzger said, I tend to think of jj commit as being the same as jj describe followed by jj new, and I don't think describe should update the author information. If we do change it, it might be good to have it be opt-in through a config option.

Are there any existing cases where the behavior is different for commit compared to desc + new?

@yuja
Copy link
Collaborator

yuja commented Jul 2, 2024

Are there any existing cases where the behavior is different for commit compared to desc + new?

No, I don't think so (except for splitting.)

I just thought it might make sense to people coming from other VCSs if commit updated author timestamp. I don't have strong feeling about that.

@scott2000
Copy link
Collaborator Author

I just thought it might make sense to people coming from other VCSs if commit updated author timestamp. I don't have strong feeling about that.

Yeah, it does seem like it would better match the expectations of new users coming from Git. One concern I have is that new users might also use jj split to replicate git commit -p and then be confused why it doesn't behave the same as jj commit.

If we update jj commit, we could also update jj split to reset the author on the first commit, but I'm not sure whether that would be surprising when splitting commits other than @, since semantically the second commit might be the "new" one.

I'm curious how many Git users actually know that there's separate author and committer timestamps. It could be the case that most people don't even know/care how it works in Git, in which case it wouldn't matter that jj behaves differently.

@PhilipMetzger
Copy link
Collaborator

I just thought it might make sense to people coming from other VCSs if commit updated author timestamp. I don't have strong feeling about that.

Yeah, it does seem like it would better match the expectations of new users coming from Git. One concern I have is that new users might also use jj split to replicate git commit -p and then be confused why it doesn't behave the same as jj commit.

Let's not change the meaning of commit to perfectly match Git, there's no reason to as Jujutsu is a separate system.

@yuja
Copy link
Collaborator

yuja commented Jul 2, 2024

One concern I have is that new users might also use jj split to replicate git commit -p and then be confused why it doesn't behave the same as jj commit.

I wouldn't consider jj split as a command to finish up the first commit, so I wouldn't expect it to update author timestamp. However, I see your point.

@emilazy
Copy link
Collaborator

emilazy commented Jul 2, 2024

I'm curious how many Git users actually know that there's separate author and committer timestamps. It could be the case that most people don't even know/care how it works in Git, in which case it wouldn't matter that jj behaves differently.

My anecdotal experience is that most Git users barely think about timestamps at all.

@scott2000 scott2000 merged commit e54ecef into martinvonz:main Jul 2, 2024
17 checks passed
@scott2000 scott2000 deleted the commit-reset-author branch July 2, 2024 23:21
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.

4 participants