Skip to content

Add worktree functionality: listing, add, remove and prune #479

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

Merged
merged 2 commits into from
Sep 5, 2020

Conversation

hatkyinc2
Copy link
Contributor

@hatkyinc2 hatkyinc2 commented Aug 1, 2020

Your checklist for this pull request

🚨Please review the guidelines for contributing to this repository.

  • Ensure all commits include DCO sign-off.
  • Ensure that your contributions pass unit testing.
  • Ensure that your contributions contain documentation if applicable.

Description

Add worktree functionality: listing, add, remove and prune

fixes #477

Checklist from feedback

  • Dedicated fixture for the worktree feature
  • README.md - Major Objects, add worktree
  • README.md - the big blob tree of read operations
  • README.md - the big blob tree of write operations
  • Squash commits

@jcouball
Copy link
Member

@hatkyinc2 thank you for the PR. The functionality looks good so far but before I study the functionality in depth, I'd get your feedback for two changes:

What do you think about creating a test fixture specifically for this change? Reusing the default test fixture (test/files/working/dot_git) as you can see breaks tests that don't have anything to do with the change you are making. To me this is tech debt that is piling up with each change like this one. The next PR might have to correct all the same tests plus the new ones introduced in this PR. Additionally, I think merge conflicts will be more of a problem if everyone is trying to change the same test fixture for their tests.

You can see an example of using a different fixture in tests/units/test_diff_non_default_encoding.rb for using the fixture tests/files/encoding

Can you minimally document this new feature in the README.md? There are two places this should be added. In the MAJOR OBJECTS section and in EXAMPLES section. Several examples will be helpful. Note any caveats.

The CONTRIBUTING guide asks for YARD documentation, but I think there needs to be across the board rework / consistency to make that useful at this point.

@jcouball
Copy link
Member

I re-ran the build and the status was updated. Not sure why it wasn't updating before.

@hatkyinc2
Copy link
Contributor Author

Glad to hear back, be happy to improve the patch.
So what I understand and am going to hack on:

  • Dedicated fixture for the worktree feature
  • README.md - Major Objects, add worktree
  • README.md - the big blob tree of read operations
  • README.md - the big blob tree of write operations

Was there other work I missed for now?

list, add, remove and prune

Signed-off-by: Ofir Petrushka <hatkyinc@gmail.com>
@hatkyinc2 hatkyinc2 changed the title Add worktree functionality: listing, add and remove Add worktree functionality: listing, add, remove and prune Aug 17, 2020
@hatkyinc2
Copy link
Contributor Author

@jcouball Updated the patch according to above, dedicated fixture, not touching current tests, added docs.

@jcouball jcouball added this to the 1.8.0 milestone Aug 17, 2020
Copy link
Member

@jcouball jcouball left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you for the contribution.

@jcouball jcouball merged commit d1908f6 into ruby-git:master Sep 5, 2020
@hatkyinc2 hatkyinc2 deleted the worktree branch September 5, 2020 23:58
@SolaWing
Copy link

@hatkyinc2 @jcouball git diff in worktree is broken. report index file open failed: Not a directory. and debug, the GIT_INDEX_FILE environment is set to wrong path

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.

Feature Request: add git worktree support
4 participants