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

LSIF: Fix empty string bug in git log parsing. #6622

Merged
merged 3 commits into from
Nov 15, 2019
Merged

Conversation

efritz
Copy link
Contributor

@efritz efritz commented Nov 15, 2019

There can be trailing whitespace in git log with ' ' indicating no parent. This isn't accounted for and returns '' rather than undefined causing a check constraint violation.

@efritz efritz added the lsif label Nov 15, 2019
@efritz efritz requested a review from a team as a code owner November 15, 2019 02:53
@codecov
Copy link

codecov bot commented Nov 15, 2019

Codecov Report

Merging #6622 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #6622      +/-   ##
=========================================
+ Coverage    39.1%   39.1%   +<.01%     
=========================================
  Files        1224    1224              
  Lines       62674   62677       +3     
  Branches     6082    6083       +1     
=========================================
+ Hits        24509   24512       +3     
  Misses      35925   35925              
  Partials     2240    2240
Impacted Files Coverage Δ
lsif/src/shared/xrepo/commits.ts 92.3% <100%> (+0.64%) ⬆️

Copy link
Contributor

@felixfbecker felixfbecker left a comment

Choose a reason for hiding this comment

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

Interesting, didn't know git put spaces there. Are you sure it was a space and not a trailing newline?

@efritz
Copy link
Contributor Author

efritz commented Nov 15, 2019

Interesting, didn't know git put spaces there. Are you sure it was a space and not a trailing newline?

The parent is violated, not the commit. To be extra sure I'll make sure to trim empty lines as well.

@efritz efritz merged commit 9381787 into master Nov 15, 2019
@efritz efritz deleted the lsif-bad-commit-values branch November 15, 2019 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants