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

Warn user about the working copy when configuring the author #4130

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

InCogNiTo124
Copy link
Contributor

@InCogNiTo124 InCogNiTo124 commented Jul 20, 2024

Closes #4042

Checklist

If applicable:

  • I have updated CHANGELOG.md

TODO

cli/src/commands/config.rs Outdated Show resolved Hide resolved
cli/src/commands/config.rs Outdated Show resolved Hide resolved
@InCogNiTo124 InCogNiTo124 force-pushed the patch-4042 branch 3 times, most recently from 80a61ec to 1628252 Compare July 21, 2024 19:48
@InCogNiTo124
Copy link
Contributor Author

Add jj git init flags for user and email

It's not clear to me how I should approach this. I noticed there's a config::ConfigBuilder but I don't think I can easily override UserSettings or RepoSettings. All suggestions welcome (including takeover from someone with more know-how 😃 )

@yuja
Copy link
Contributor

yuja commented Jul 23, 2024

Add jj git init flags for user and email

It's not clear to me how I should approach this.

I personally don't think this is good idea, but if needed, it's probably easier to implement as global flags. (See how --color is translated to config value.)

@InCogNiTo124
Copy link
Contributor Author

if needed, it's probably easier to implement as global flags

I would argue that that's not a clean solution, either, since those flags are only needed for jj git init

@yuja
Copy link
Contributor

yuja commented Jul 23, 2024

if needed, it's probably easier to implement as global flags

I would argue that that's not a clean solution, either, since those flags are only needed for jj git init

I'm not sure why it's needed for (and should be restricted to) jj git init. User name/email applies globally, so it might make sense to do jj describe --user foo. (To be clear, I don't think the global flag is needed because it can be achieved by --config-toml ...)

@InCogNiTo124 InCogNiTo124 force-pushed the patch-4042 branch 2 times, most recently from 16add06 to a84113c Compare August 4, 2024 21:20
@InCogNiTo124 InCogNiTo124 force-pushed the patch-4042 branch 2 times, most recently from 2345152 to baa308c Compare August 6, 2024 19:20
@InCogNiTo124
Copy link
Contributor Author

Tested with following snippets that it works:

Testing snippet
JJ_USER=user1 JJ_EMAIL=email1 jj git init .
Initialized repo in "."jj log
@  smytynvq email1 2024-08-06 21:11:21 303fc106
│  (empty) (no description set)
◆  zzzzzzzz root() 00000000jj config set --repo user.email "email2"
Warning: This setting will only impact future commits.
The author of the working copy will stay "user1 <email1>". 
To change it, use "jj describe --reset-author --no-edit"jj log
@  smytynvq email1 2024-08-06 21:11:21 303fc106
│  (empty) (no description set)
◆  zzzzzzzz root() 00000000jj describe --reset-author --no-edit
Working copy now at: smytynvq 1f2c39c8 (empty) (no description set)
Parent commit      : zzzzzzzz 00000000 (empty) (no description set)jj log
@  smytynvq email2 2024-08-06 21:12:08 1f2c39c8
│  (empty) (no description set)
◆  zzzzzzzz root() 00000000echo 'a' > a; jj commit -m 'a'; jj log
Working copy now at: xywsopol 5973def1 (empty) (no description set)
Parent commit      : smytynvq 92a5caae a
@  xywsopol email2 2024-08-06 21:12:45 5973def1
│  (empty) (no description set)
○  smytynvq email2 2024-08-06 21:12:45 92a5caae
│  a
◆  zzzzzzzz root() 00000000jj config set --repo user.email "[email protected]"
Warning: This setting will only impact future commits.
The author of the working copy will stay "InCogNiTo124 <email2>". 
To change it, use "jj describe --reset-author --no-edit"jj describe --no-edit --reset-author 'author("email2")'; jj log
Updated 2 commits
Working copy now at: xywsopol cbae64d7 (empty) (no description set)
Parent commit      : smytynvq c8442055 a
@  xywsopol [email protected] 2024-08-06 21:13:53 cbae64d7
│  (empty) (no description set)
○  smytynvq [email protected] 2024-08-06 21:13:53 c8442055
│  a
◆  zzzzzzzz root() 00000000

@InCogNiTo124 InCogNiTo124 marked this pull request as ready for review August 6, 2024 19:37
@InCogNiTo124 InCogNiTo124 force-pushed the patch-4042 branch 6 times, most recently from 84a34ba to 01197eb Compare August 12, 2024 19:49
Copy link
Contributor

@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.

Thanks!

cli/src/commands/config/set.rs Outdated Show resolved Hide resolved
cli/src/commands/config/set.rs Outdated Show resolved Hide resolved
cli/src/commands/config/set.rs Show resolved Hide resolved
cli/src/commands/config/set.rs Outdated Show resolved Hide resolved
cli/src/commands/config/set.rs Outdated Show resolved Hide resolved
cli/src/commands/config/set.rs Outdated Show resolved Hide resolved
@InCogNiTo124 InCogNiTo124 force-pushed the patch-4042 branch 4 times, most recently from 497d19b to b8f73e8 Compare August 13, 2024 18:55
cli/src/commands/config/set.rs Outdated Show resolved Hide resolved
cli/src/commands/config/set.rs Outdated Show resolved Hide resolved
cli/src/commands/config/set.rs Outdated Show resolved Hide resolved
cli/src/commands/config/set.rs Show resolved Hide resolved
cli/tests/test_config_command.rs Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@InCogNiTo124 InCogNiTo124 force-pushed the patch-4042 branch 2 times, most recently from c630816 to 5066d08 Compare August 14, 2024 20:44
@InCogNiTo124 InCogNiTo124 force-pushed the patch-4042 branch 2 times, most recently from c0b3611 to f095fec Compare August 14, 2024 20:53
cli/src/commands/config/set.rs Outdated Show resolved Hide resolved
@InCogNiTo124 InCogNiTo124 force-pushed the patch-4042 branch 2 times, most recently from 03c0982 to 72e28f3 Compare August 16, 2024 14:31
@InCogNiTo124
Copy link
Contributor Author

@yuja I believe this can be merged

@yuja
Copy link
Contributor

yuja commented Aug 19, 2024

@yuja I believe this can be merged

Yes. Maybe you don't have a permission to merge the PR yet?

cc @martinvonz

@martinvonz
Copy link
Member

@yuja I believe this can be merged

Yes. Maybe you don't have a permission to merge the PR yet?

Yes, that's correct. Sorry that I didn't realize :(

@InCogNiTo124 InCogNiTo124 merged commit 0852724 into jj-vcs:main Aug 19, 2024
17 checks passed
@InCogNiTo124 InCogNiTo124 deleted the patch-4042 branch August 19, 2024 15:09
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.

Surprising author of the first working copy
4 participants