-
Notifications
You must be signed in to change notification settings - Fork 533
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
Conversation
@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 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. |
I re-ran the build and the status was updated. Not sure why it wasn't updating before. |
Glad to hear back, be happy to improve the patch.
Was there other work I missed for now? |
list, add, remove and prune Signed-off-by: Ofir Petrushka <hatkyinc@gmail.com>
@jcouball Updated the patch according to above, dedicated fixture, not touching current tests, added docs. |
There was a problem hiding this 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.
@hatkyinc2 @jcouball git diff in worktree is broken. report |
Your checklist for this pull request
🚨Please review the guidelines for contributing to this repository.
Description
Add worktree functionality: listing, add, remove and prune
fixes #477
Checklist from feedback