search

orientechnologies / orientdb-gremlin

TinkerPop3 Graph Structure Implementation for OrientDB
Apache License 2.0
91 stars 32 forks source link

Creating vertex label with correct prefix #89

Closed fppt closed 8 years ago

fppt commented 8 years ago

Using the method createVertexClass() required you to prepend "V_" yourself I thought maybe having a new method for creating labels this way could be useful.

fppt commented 8 years ago

I am not sure why these changes lead to failures. As I am very new to this project could someone point me in the right direction for fixing them ? Is there something simple missing on the scala side ?

mpollmeier commented 8 years ago

Thank you! You can ignore appveyor (I do anyway... any thoughts @velo ?) travis-ci enforces the formatting guidelines, that's what it fails with if you look at the logs. The readme explains how to easily fix this.

Re your implementation: as you figured out in https://github.com/mpollmeier/orientdb-gremlin/issues/88 this is a bug, and since label and vertexClass/edgeClass mean basically the same thing, can you undo your changes in OrientGraph and simply replace className with labelToClassName in createVertexClass/createEdgeClass? Keep the test, just make it compile again by calling these existing methods.

If this is confusing just let me know and I'll do it quickly.

fppt commented 8 years ago

Thank you for the guidance. I think I made the changes you suggested, unfortunately some other tests started failing so I needed to make more changes. Please let me know if I took too many liberties with the code.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 84.221% when pulling 7a7d194b81be1f29c82998d6df7e2c73fd40c753 on fppt:class-name-bug-fix into 742c2d089b97d7af9118349230cc63393bbae50f on mpollmeier:master.

mpollmeier commented 8 years ago

looks great, thank you! would you like me to release a new version?

fppt commented 8 years ago

No need, I am happy to keep working with the snapshot version until another release is made.

Why did you close this PS though. Is something else required to merge it in ?

mpollmeier commented 8 years ago

sorry, merged just now :)

fppt commented 8 years ago

Great thank you.