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

api: JSON omitempty on optional fields in FileMatch #721

Merged
merged 1 commit into from
Jan 18, 2024
Merged

Conversation

keegancsmith
Copy link
Member

There are several fields which are optional in FileMatch. For example branch and version are only set if indexing a git repo (zoekt can index arbitrary files). We mark these fields as optional to marshal in JSON. The main benefit of this is improving the readability of some golden files.

Note: I noticed this when an unrelated commit touched these golden files.

Test Plan: go test

There are several fields which are optional in FileMatch. For example
branch and version are only set if indexing a git repo (zoekt can index
arbitrary files). We mark these fields as optional to marshal in JSON.
The main benefit of this is improving the readability of some golden
files.

Note: I noticed this when an unrelated commit touched these golden
files.

Test Plan: go test
Copy link
Member

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

Makes sense to me! Do we want to do anything similar for the FileMatch structs in sourcegraph/sourcegraph? (Maybe not, they look pretty streamlined to me already)

@keegancsmith keegancsmith merged commit 18b6999 into main Jan 18, 2024
8 checks passed
@keegancsmith keegancsmith deleted the k/omitempty branch January 18, 2024 03:59
@keegancsmith
Copy link
Member Author

Do we want to do anything similar for the FileMatch structs in sourcegraph/sourcegraph? (Maybe not, they look pretty streamlined to me already)

Yeah it looks pretty minimal to me.

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.

2 participants