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

Add flag to support for CQL #31

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gamorejon
Copy link

Add support for CQL which is used by Cassandra, ScyllaDB, YugeByte, etc. Works similarly to json in that it adds Marshal and Unmarshal methods.

@gamorejon gamorejon closed this Mar 3, 2020
@dmarkham
Copy link
Owner

@gamorejon were you not happy with this code?

@gamorejon
Copy link
Author

There was a bug in the generated code that I fixed, but it's all working now and we are using in production.
Mostly, I was unsure if you were open to taking it because adding the extra flag to main seemed to tip your code coverage tool over the threshold. I'll re-open if you are.

@dmarkham
Copy link
Owner

When I look though the code, it looked just fine I was more concerned about the introduction of a 3rd party include github.com/gocql/gocql currently I think everything we use is internal, my luck the next person will want to add some other cql library! Really wish the UnmarshalCQL did not require gocql.TypeInfo 😢 I guess if you have it all working and your happy with it, open it up maybe I can take some time try to get the coverage figured out.

@gamorejon
Copy link
Author

Will do. Yeah, the dependency is not ideal -- especially since we don't care about supporting non-string CQL types -- but if it makes you feel better gocql is a low-level package for CQL which all the other ORM-like packages depend on. It's unlikely to be displaced for at least for a few years and TypeInfo is an interface so things still work with any forks. The ScyllaDB guys (they are a Go shop, for everything except the core DB) maintain a fork of gocql for gocqlx which we use and there are no issues.

@gamorejon gamorejon reopened this Mar 19, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #31 into master will increase coverage by 0.00%.
The diff coverage is 57.14%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #31   +/-   ##
=======================================
  Coverage   66.14%   66.15%           
=======================================
  Files           3        3           
  Lines         449      455    +6     
=======================================
+ Hits          297      301    +4     
- Misses        140      142    +2     
  Partials       12       12           
Impacted Files Coverage Δ
stringer.go 61.11% <40.00%> (-0.12%) ⬇️
enumer.go 100.00% <100.00%> (ø)
sql.go 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d46c853...008ac77. Read the comment docs.

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.

None yet

3 participants