Skip to content

Move filters out tree nodes, rather than delete their sha - #215

Open
mushishi78 wants to merge 1 commit into
github-tools:masterfrom
mushishi78:move-delete-tree-node
Open

Move filters out tree nodes, rather than delete their sha#215
mushishi78 wants to merge 1 commit into
github-tools:masterfrom
mushishi78:move-delete-tree-node

Conversation

@mushishi78

Copy link
Copy Markdown
Contributor

I was getting an error when moving files, and I came across this issue with a suggested workaround that worked for me. So I thought I'd send over a PR just incase.

However, moving files was working for me just fine before, and so the real cause of the problem is a mystery as far as I concerned. If no-one else is having the problem, then I understand if you don't feel a need to merge.

@mushishi78mushishi78 changed the title Move filters out tree nodes, rather than delete their refMove filters out tree nodes, rather than delete their shaAug 29, 2015
@aendra-rininsland

Copy link
Copy Markdown
Member

Hi @mushishi78!

Could you please write a unit test for this? That would really help in determining whether this is a valid issue or not. I suggest writing the test case and running it against master HEAD and your PR; should fail in the former and pass in the latter. Thank you!

@mushishi78

Copy link
Copy Markdown
ContributorAuthor

Sorry to disappoint you again, but I also don't know how to write a test for this either. As far as I can tell, there is no previously written test for the move function, and even if their was one, I'd need to understand what caused the bug in order to write a test that isolated the bug.

To write a test for the move function, I'd think you'd need a to do some set up code for your testing that would create a test branch on a test repo and then delete the branch afterwards. That way you'd be free to make changes to without repercussion, you could rely on the same files being present in the repo and I could move some files or fail to move some files around with reckless abandon.

@aendra-rininslandaendra-rininsland modified the milestones: v0.10.8, v0.10.7Oct 30, 2015
@aendra-rininsland

Copy link
Copy Markdown
Member

Bumping to 0.10.8, we really need to release 0.10.7.

@aendra-rininsland

Copy link
Copy Markdown
Member

Closed in error; reopening.

@aendra-rininsland
aendra-rininslandforce-pushed the master branch 2 times, most recently from e746e5d to 6f04f13CompareNovember 17, 2015 16:15
@AurelioDeRosaAurelioDeRosa modified the milestones: v0.11.0, v0.10.8, 0.12.0, 0.11.0Jan 24, 2016
@clayreimannclayreimann modified the milestones: 1.x, 0.12.0Apr 27, 2016
@AurelioDeRosa

Copy link
Copy Markdown
Member

@clayreimann I think this PR can improve the lib as it sends only the data needed. Should I rebase it?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@mushishi78@aendra-rininsland@AurelioDeRosa@kyheng@clayreimann