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

updates to get everything working with go 1.18 #718

Closed

Conversation

adamstruck
Copy link
Contributor

@adamstruck adamstruck commented Jul 12, 2023

Works for me - thought it could be useful given the ongoing compliance work I have seen.

@kellrott
Copy link
Contributor

We've been working on getting up to TES 1.1 compliance. We'll need to review this against all the changes in #716

@lbeckman314
Copy link
Member

lbeckman314 commented Jul 12, 2023

This is great! We've just recently been getting past a possible logging concurrency bug* with MongoDB and Badger that has been blocking the PR in #716. I'll try rebasing that branch and this one to reconcile the changes into a mergeable PR and will update here for any review or recommendations!


*Specifically with the EventWriter attempting to write the last event to the database after the connection has been closed. fix/lint adds a WaitGroup to block the logging in Run() before closing.

@adamstruck
Copy link
Contributor Author

Looks like this PR can probably just be closed in favor of #716.

You may want to use the ‘.golangci.yml’ file and compare the proto code generation command in the Makefile.

@lbeckman314 lbeckman314 mentioned this pull request Jul 29, 2023
@lbeckman314
Copy link
Member

Cleaning up PR's but making a note of this to revisit the Proto discussion and proto code generation as that could definitely be improved on now that compliance tests are happy.

Looks like this PR can probably just be closed in favor of #716.

You may want to use the ‘.golangci.yml’ file and compare the proto code generation command in the Makefile.

We're currently manually patching the tes.proto file with explicit views (minimal, basic, full) and using a custom marshaller (marshal.go) in order to get around the problem of protobuf not emitting empty slices (marshal_jsonpb.go).

This could be improved in a couple ways, including:

  1. Updating the proto code generator to include the the views enum value
  2. Replacing our marshal.go with an updated version of marshal_jsonpb.go that outputs empty slices

Updating the code generator would very likely be the simplest option with implementing the custom marshaler being a backup.

@lbeckman314 lbeckman314 closed this Dec 7, 2023
@lbeckman314 lbeckman314 mentioned this pull request Jan 27, 2024
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.

3 participants