Gazler / githug

Git your game on!
MIT License
6.86k stars 1.03k forks source link

[Contribution] New level #146

Closed martinmose closed 10 years ago

martinmose commented 10 years ago

Hello,

First of all thanks for a wonderful "game" - I just wanted to make a contribution to show my support for the game - I hope it is somehow useful.

I have wrote a level, and tested that every thing is working :). I'm new to Git - that's why I was going through Githug - which was awesome! And it's my first pull request too... So I'm not sure if I have done it correctly, please tell me if not and hopefully I can make it up :+1:

martinmose commented 10 years ago

@MarkJessop Thanks for your reply.

Hmm that didn't make any differents :/. I can also see many of the other test cases are using the "pipe" annotation - and I can't see why it shouldn't work! But now my RSpec stop "responding" when I'm trying to run the tests locally.

I cannot see how my changes should have broken the build...

And now when I think about it, my test case should might have been:

    `echo "2"`
    `githug`.should be_solved

and maybe I could add:

    `git ls-files --other --exclude-standard`

But I will not commit anything, until I can test it locally again.

Gazler commented 10 years ago

@martinmose When you are doing:

`git ls-files --other --exclude-standard | githug`.should be_solved

It is piping rubyfile6.rb\nrubyfile7.rb into githug, when githug is expecting the value 2. I propose the following solutions:

`git diff --name-only --cached | wc -l | githug`.should be_solved

Or:

`echo "2" | githug`.should be_solved

I like the first one, but you decide.

Also, I would appreciate it if you could squash your commits for this pull request, it looks like a bit of trial and error has been going on and it would be neater in the commit history as a single commit. If you are not comfortable doing that, I can do it when I merge it in.

Thanks, Gazler.

martinmose commented 10 years ago

@Gazler Thank you, for pointing that out.

I would indeed prefer:

`git diff --name-only --cached | wc -l | githug`.should be_solved

I didn't think of that.

Yes it got a bit "messy" because I was messing around with git :P. I actually tried to combine the commit's into one, with git rebase... so I could get rid of all the "test" commits - it didn't work out very well, as you can see. If you would do that, that would be awesome.

Gazler commented 10 years ago

@martinmose If you update the test then I'll sort the commits when merging it in.

Gazler commented 10 years ago

@martinmose This is now merged. Sorry it took so long!

martinmose commented 10 years ago

No Problem, I'm just happy I could contribute with something! :)