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

cmd: --branch option for git fetch. #1146

Merged
merged 3 commits into from
Feb 22, 2023
Merged

cmd: --branch option for git fetch. #1146

merged 3 commits into from
Feb 22, 2023

Conversation

ilyagr
Copy link
Collaborator

@ilyagr ilyagr commented Jan 29, 2023

The main contribution here are the detailed tests.

There is an outstanding bug with the way undo interacts with jj git fetch --branch.

Checklist

Todo:

  • I have updated CHANGELOG.md
  • (not planned) I have updated the documentation (README.md, docs/, demos/)
  • I have added tests to cover my changes

tests/test_git_fetch.rs Outdated Show resolved Hide resolved
tests/test_git_fetch.rs Outdated Show resolved Hide resolved
@ilyagr ilyagr force-pushed the ig/fetchglob branch 2 times, most recently from 560f761 to c9ab741 Compare January 29, 2023 06:03
lib/src/git.rs Outdated Show resolved Hide resolved
lib/src/git.rs Show resolved Hide resolved
lib/src/git.rs Outdated Show resolved Hide resolved
@ilyagr
Copy link
Collaborator Author

ilyagr commented Feb 12, 2023

FYI, I have a synced-to-head version of this at https://github.com/ilyagr/jj/tree/fetch-glob.

The test there is a union of the test from head and my test, and needs to be merged more intelligently.

It's buggy enough that I couldn't put it here easily (I haven't tried doing jj git fetch --remote upstream without --glob yet).

$ jj git push --remote upstream -b ig/fetchglob
Branch changes to push to upstream:
  Add branch ig/fetchglob to c20c34f0e914
Error: Push is not fast-forwardable

@ilyagr ilyagr force-pushed the ig/fetchglob branch 5 times, most recently from 7c9efa6 to 7234fc4 Compare February 20, 2023 03:31
@ilyagr
Copy link
Collaborator Author

ilyagr commented Feb 20, 2023

There are a couple of known outstanding bugs (search for BUG in the tests). However, I think it's better to check this in as is rather than leave it hanging. I'll file these bugs as I submit this.

@ilyagr ilyagr marked this pull request as ready for review February 20, 2023 03:39
@ilyagr ilyagr changed the title cmd: --glob option for git fetch. cmd: --branch option for git fetch. Feb 20, 2023
@ilyagr ilyagr force-pushed the ig/fetchglob branch 3 times, most recently from 64c4325 to fae335e Compare February 20, 2023 08:40
Copy link
Collaborator

@samueltardieu samueltardieu left a comment

Choose a reason for hiding this comment

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

I am all for including this soon, this is a feature I will use very often when pulling a pull request from a third-party repository without getting the whole content.

There is one thing I'm unsure about (the refspec) and I've let a comment on it, I would be more confident if you included the output of get_branch_output() at several points in the test.

lib/src/git.rs Outdated Show resolved Hide resolved
@ilyagr ilyagr force-pushed the ig/fetchglob branch 5 times, most recently from 799b1c6 to 7deb556 Compare February 20, 2023 21:35
@ilyagr ilyagr force-pushed the ig/fetchglob branch 3 times, most recently from d707573 to 525819f Compare February 21, 2023 04:35
CHANGELOG.md Outdated Show resolved Hide resolved
Thanks to @samueltardieu for noticing a subtle bug in the refspecs, providing
the fix, as well as the two `conflicting_branches` tests.
@ilyagr ilyagr enabled auto-merge (rebase) February 22, 2023 02:20
@ilyagr
Copy link
Collaborator Author

ilyagr commented Feb 22, 2023

Thanks Samuel!

@ilyagr ilyagr merged commit 643fe9a into main Feb 22, 2023
@ilyagr ilyagr deleted the ig/fetchglob branch February 22, 2023 02:33
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