Skip to content

internal/function: use custom fixed struct as LOC result #931

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

Merged
merged 2 commits into from
Jul 30, 2019

Conversation

erizocosmico
Copy link
Contributor

Fixes #927

  • I updated the documentation explaining the new behavior if any.
  • I updated CHANGELOG.md file adding the new feature or bug fix.
  • I updated go-mysql-server using make upgrade command if applicable.
  • I added or updated examples if applicable.
  • I checked that changes on schema are reflected into the documentation, if applicable.

@erizocosmico erizocosmico requested a review from a team July 11, 2019 15:30
@@ -81,9 +81,9 @@ GROUP BY committer_email,
```sql
SELECT
LANGUAGE(file_path, blob_content) as lang,
SUM(JSON_EXTRACT(LOC(file_path, blob_content), '$.Code')) as code,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say to do the opposite, keep output as it was. Also, other functions have keys with first letter uppercase too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using uppercase is a bit weird in JSON, hence #930

If we do want to use uppercase, though, I can close the issue and change it back here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm concerned about backward compatibility. All queries that are using these functions will stop working with no error message...

@erizocosmico
Copy link
Contributor Author

Struct tags are now uppercase. Docs added with the shape of LOC.

@erizocosmico erizocosmico force-pushed the fix/loc-fixed-result branch from 1a89332 to 499c7d5 Compare July 29, 2019 08:05
@erizocosmico
Copy link
Contributor Author

Done

@ajnavarro ajnavarro requested a review from juanjux July 29, 2019 14:25
@ajnavarro ajnavarro merged commit 43c2c25 into src-d:master Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LOC function is returning external library object that might change
3 participants