Skip to content

Modify proto with ChangeRequest.TwoDotsMode#76

Open
dpordomingo wants to merge 1 commit intosrc-d:masterfrom
dpordomingo:add-merge-base
Open

Modify proto with ChangeRequest.TwoDotsMode#76
dpordomingo wants to merge 1 commit intosrc-d:masterfrom
dpordomingo:add-merge-base

Conversation

@dpordomingo
Copy link
Copy Markdown
Contributor

blocks src-d/lookout#530

In order to show the same changes than appear in Changes tab from GitHub PRs, the standard behavior for DataService.GetChanges is returning the changes as if doing git diff base...head (what works as git diff $(git merge-base base head) head)

If the analyzer wants all changes between base and head, (as done by git diff base..head) it must send TwoDotsMode as true in the ChangesRequest.

Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
@dpordomingo dpordomingo requested a review from carlosms February 21, 2019 09:48
@dpordomingo dpordomingo self-assigned this Feb 21, 2019
@dpordomingo dpordomingo added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Feb 21, 2019
Copy link
Copy Markdown
Contributor

@carlosms carlosms left a comment

Choose a reason for hiding this comment

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

👍

But let's wait for src-d/lookout#530 to be finished before we merge and release this one.

@dpordomingo
Copy link
Copy Markdown
Contributor Author

It would be great if we could release this before merging src-d/lookout#530 to see Travis pass 💃
I know that if we merge this before the other, the analyzers will still having the same bug that previously, but since it's its current behavior... wdyt?

@carlosms
Copy link
Copy Markdown
Contributor

Sorry, I didn't mean to merge src-d/lookout#530 before this one. What I meant is to:

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

Labels

blocked bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants