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

cli: list new remote branches during git fetch #3044

Merged
merged 1 commit into from
Feb 18, 2024
Merged

Conversation

0xdeafbeef
Copy link
Member

@0xdeafbeef 0xdeafbeef commented Feb 14, 2024

Adds imported branches list for git fetch.

jj git fetch
From origin
* [new untracked branch]  test2

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

@0xdeafbeef
Copy link
Member Author

I'm not sure what to do with these tests. They are designed to expect no stdout in the event of an error. I can address this issue, but I'm unsure if I should.

@yuja
Copy link
Contributor

yuja commented Feb 14, 2024

I'm not sure what to do with these tests. They are designed to expect no stdout in the event of an error.

Perhaps, the output can be written to stderr. It's kind of a status message.

@0xdeafbeef
Copy link
Member Author

I'm not sure what to do with these tests. They are designed to expect no stdout in the event of an error.

Perhaps, the output can be written to stderr. It's kind of a status message.

I hadn't noticed that 'Abandoned commits' were already being written to stderr. With these changes, all tests now pass.

@0xdeafbeef 0xdeafbeef marked this pull request as ready for review February 14, 2024 15:45
@0xdeafbeef 0xdeafbeef requested a review from yuja February 14, 2024 15:45
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!

lib/src/git.rs Outdated Show resolved Hide resolved
lib/src/git.rs Outdated Show resolved Hide resolved
cli/src/git_util.rs Outdated Show resolved Hide resolved
cli/src/git_util.rs Outdated Show resolved Hide resolved
cli/src/git_util.rs Outdated Show resolved Hide resolved
cli/src/git_util.rs Outdated Show resolved Hide resolved
cli/src/commands/git.rs Outdated Show resolved Hide resolved
@0xdeafbeef 0xdeafbeef force-pushed the push-ppomxyyutuqm branch 2 times, most recently from 9a97a3f to f30144a Compare February 16, 2024 23:08
cli/src/git_util.rs Show resolved Hide resolved
cli/src/git_util.rs Outdated Show resolved Hide resolved
cli/src/git_util.rs Outdated Show resolved Hide resolved
cli/src/git_util.rs Show resolved Hide resolved
cli/src/commands/git.rs Outdated Show resolved Hide resolved
cli/src/commands/git.rs Outdated Show resolved Hide resolved
@0xdeafbeef 0xdeafbeef force-pushed the push-ppomxyyutuqm branch 3 times, most recently from f6dec62 to 62e593f Compare February 17, 2024 16:53
@0xdeafbeef
Copy link
Member Author

I'm not sure what to do with reference names that are too long. For now, padding is limited to 40 characters; perhaps we should consider using some kind of ellipsis

@martinvonz
Copy link
Member

I'm not sure what to do with reference names that are too long. For now, padding is limited to 40 characters; perhaps we should consider using some kind of ellipsis

Seems fine for now. Padding to accommodate the longer name also seems fine.

@0xdeafbeef
Copy link
Member Author

I've added color for branch/tag name, alignment if both tags and branches are present.
image

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.

Looks mostly good, thanks!

cli/src/commands/git.rs Outdated Show resolved Hide resolved
cli/src/commands/git.rs Outdated Show resolved Hide resolved
cli/src/commands/git.rs Outdated Show resolved Hide resolved
cli/src/git_util.rs Outdated Show resolved Hide resolved
cli/src/git_util.rs Outdated Show resolved Hide resolved
cli/src/commands/git.rs Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@0xdeafbeef
Copy link
Member Author

@yuja, I've implemented all the suggested changes, and it looks like the PR has received your approval. Unless there are any further comments or adjustments needed, may I proceed with merging it? Thank you for your guidance!

@0xdeafbeef 0xdeafbeef merged commit 06d67f0 into main Feb 18, 2024
15 checks passed
@0xdeafbeef 0xdeafbeef deleted the push-ppomxyyutuqm branch February 18, 2024 16:36
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.

3 participants