Add support for comparing two commits #428
Conversation
shiftkey
commented
Mar 8, 2014
shiftkey
commented
- add interface, implementation and response
- add reactive implementation
- unit tests
- convention tests
- integration tests
- update commit model to use what's in Add comparing commits tags branches #433
- tighten up parameter validation based on feedback
|
Addresses one of the features in #333 |
|
Right, review away |
|
Have you testet your compare on the live API? Since your datamodel is not correct. I startet to make mine the same way but the commit model is not good enough :) |
|
@hknielsen I have integration tests in here which use the live real API. I'm happy to refine that model in another PR, just wanted to flag the possible duplication here. |
|
Added task to update commit model returned from API |
|
@shiftkey As I can see you just count the commits, but not whats in them. The Commit model has a extra level, so you wont get the message or tree. You could just add it here, since it does not really show anything about the commit besides the sha that you can use? |
|
@shiftkey If you wont do it, I can just add it based on you pull request? :) the problem is I need the messages in the commit, thats why im so focust about it |
|
@hknielsen all good, I've made that change here. Let me know if there's anything else I've overlooked here... |
|
👍 thanks |
| public Commit Commit { get; set; } | ||
| public Author Committer { get; set; } | ||
| public string HtmlUrl { get; set; } | ||
| public IReadOnlyList<GitReference> Parents { get; set; } |
|
When will this get merged? |
|
Let everyone sleep on it. I don't think there'll be any sudden issues... On Wed, Mar 19, 2014 at 9:33 PM, Harald Kjær Nielsen <
|
| return _client.Repository.Commits.Compare(owner, name, @base, head).ToObservable(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
^^^ 
(I actually don't care. Just wondering if you do.)
There was a problem hiding this comment.
That little red mark there means the file doesn't end with a line ending. Some folks really care about that sort of thing. Others don't. it's like removing and sorting namespaces. I always do it, but I don't block a review for it. ;)
There was a problem hiding this comment.
One of the reasons for needing the line ending is so you can stage lines and hunks in git gui, if there is no line endings it will give an error.
There was a problem hiding this comment.
Right, so the PR summary screen is trolling me again:
There was a problem hiding this comment.
LOL Here's what I seey
One of the reasons for needing the line ending is so you can stage lines and hunks in git gui, if there is no line endings it will give an error.
That's just horrible. Git Gui should handle that.
There was a problem hiding this comment.
Yes I agree, have been thinking about making one that works and is better then that awfull tool :)
In my team we have made stylecop apart of the solution build so it always checks for the lineending, dont know if code analyzer have the same option but its maybe its something to add
|
So good. Just some really nitpicky shit. Also, it's not automatically mergeable. 😄 |
|
@haacked should be all sorted, let me know if I've missed anything... |
Add support for comparing two commits
|
Great stuff! |
