cli: print commits abandoned by git::import_refs() or fetch() #2311

yuja · 2023-09-30T08:31:27Z

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

martinvonz

Thanks! This seems useful.

By the way, one advantage of having jj git sync command is that it can avoid showing some of the conflicted commits to the user.

cli/src/cli_util.rs

cli/tests/test_git_push.rs

It'll be reported to user.

This wasn't a problem before, but we wouldn't want to report previously-hidden commits as abandoned.

I'll make it a bit more verbose if mut_repo().has_changes().

This will probably help to understand why you've got conflicts after fetching. Maybe we can also report changed local refs. I think the stats should be redirected to stderr, but we have many other similar messages printed to stdout. I'll probably fix them all at once later.

cli/src/cli_util.rs

…git import Otherwise, it's unclear why "jj status" abandoned commits for example.

ilyagr

I'm still a bit uneasy about this (whether it will create confusion, especially for beginners that didn't notice the abandoned commits and don't quite understand what it means), but this is clearly useful for people who notice the abandoned commits themselves. I think there's a good chance it's fine. We should try this version and see how (and whether) people react.

Yuya mentioned something intersting about this triggering unexpected test changes. I'll leave it up to him whether to look into it before or after merging this.

ilyagr · 2023-10-02T18:20:38Z

Here's the first time this PR became really noticeable. Most (all?) of these branches were moved, not deleted. I can't yet decide whether I like it or whether it will confuse people. We'll see :)

martinvonz approved these changes Sep 30, 2023

View reviewed changes

ilyagr reviewed Sep 30, 2023

View reviewed changes

cli/src/cli_util.rs Show resolved Hide resolved

ilyagr reviewed Sep 30, 2023

View reviewed changes

cli/tests/test_git_push.rs Show resolved Hide resolved

yuja force-pushed the push-lovswysxspvs branch from 5dda81f to 54143c8 Compare October 1, 2023 08:56

yuja added 4 commits October 2, 2023 03:22

git: make import_refs() return abandoned commits to caller

aff56c9

It'll be reported to user.

git: exclude hidden commits from list of commits to be abandoned

4a70231

This wasn't a problem before, but we wouldn't want to report previously-hidden commits as abandoned.

cli: return early if git::import_some_refs() is noop

af81d2f

I'll make it a bit more verbose if mut_repo().has_changes().

ilyagr reviewed Oct 2, 2023

View reviewed changes

cli/src/cli_util.rs Outdated Show resolved Hide resolved

cli: add context to checkout/abandon messages triggered by automated …

e301b4a

…git import Otherwise, it's unclear why "jj status" abandoned commits for example.

yuja force-pushed the push-lovswysxspvs branch from 54143c8 to e301b4a Compare October 2, 2023 04:27

ilyagr approved these changes Oct 2, 2023

View reviewed changes

yuja merged commit 480efc1 into jj-vcs:main Oct 2, 2023
15 checks passed

yuja deleted the push-lovswysxspvs branch October 2, 2023 08:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: print commits abandoned by git::import_refs() or fetch() #2311

cli: print commits abandoned by git::import_refs() or fetch() #2311

yuja commented Sep 30, 2023

martinvonz left a comment

ilyagr left a comment •

edited

Loading

ilyagr commented Oct 2, 2023 •

edited

Loading

cli: print commits abandoned by git::import_refs() or fetch() #2311

cli: print commits abandoned by git::import_refs() or fetch() #2311

Conversation

yuja commented Sep 30, 2023

Checklist

martinvonz left a comment

Choose a reason for hiding this comment

ilyagr left a comment • edited Loading

Choose a reason for hiding this comment

ilyagr commented Oct 2, 2023 • edited Loading

ilyagr left a comment •

edited

Loading

ilyagr commented Oct 2, 2023 •

edited

Loading