Skip to content
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

Modify proto with ChangeRequest.TwoDotsMode #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dpordomingo
Copy link
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 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
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
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
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.

2 participants