search

thurstonsand / scala-cass

a wrapper for the Java Cassandra driver that allows extraction from a Row with Scala types, or directly into a case class. Also has utility functions for the Session to read/write to Cassandra directly to/from a case class.
MIT License
21 stars 10 forks source link

implement https://github.com/thurstonsand/scala-cass/issues/4 #49

Open holinov opened 5 years ago

holinov commented 5 years ago

My try to implement #4

thurstonsand commented 5 years ago

this looks pretty good, but can you remove all of the formatting changes?

thurstonsand commented 5 years ago

also, i fixed some stuff related to the travis build, so if you merge in master, you should see tests actually run.

unfortunately...it seems like some difference between oracle jdk8 and openjdk8 is causing some sort of race condition in the test instance of cassandra. i need to address that, but for now, you may need to run the travis instances multiple times (by "restart job" option) to see if you can escape the race condition

holinov commented 5 years ago

  1. about formatting - i'm intended to use corp-level style assistants and dissencouraged to use alternate (if it makes real trouble i'll try to switch to different PC)

  2. about this looks pretty good - it looks terrible (i event didn't get if my queries are cached cached as prepeared)

  3. about travis - wtf? what do you mean me to do?

  4. would it make sense if i'll implement Null/NonNull/ContainsIn in this kind of adt? it would be much easier if i'll have some ideas how this should be "used idiomatic way" )))

thurstonsand commented 5 years ago

  1. im...surprised you cant just turn off styling for this particular project. It makes it hard to understand what changes you've actually made, since there's so many no-op line changes. I just want to see the parts of code that you've modified/added.

  2. looks like you modeled the code after what I did with Nullable, which seems about right. you should model your test after UpdateBehaviorTests since that seems pretty self-contained

  3. well, you merged master, so that's all i wanted you to do there about travis. you can see in the job your test is failing

  4. If I understand what you're asking, then yeah you should model this the same way that Null is done. which you pretty much have. like i said, it seems about right, but I'll take a closer look if you can remove all the random formatting changes