jhipster / jhipster-core

JHipster Domain Language, used by JHipster UML and JDL-Studio to generate entities
Apache License 2.0
346 stars 116 forks source link

Allow using UUID for all databases, not just Cassandra #331

Closed murdos closed 5 years ago

murdos commented 5 years ago

Please make sure the below checklist is followed for Pull Requests.

MathieuAA commented 5 years ago

LGTM, the last PR you need create is the one for the docs.

murdos commented 5 years ago

How can we move forward on the generator-jhipster PR and have integration tests green? It seems we need this PR merged and new JCore release, no?

MathieuAA commented 5 years ago

@murdos I don't see why. If we merge and release, anyone can write a perfectly good JDL with the new UUID support and get thrown an error when dealing with the generator. But I see you've written a test using the UUID type... You're doing dependency testing, here. Even though I'm generally okay with those, here it's mostly useless as JCore is not some third-party lib :)

pascalgrimaud commented 5 years ago

If we merge and release, anyone can write a perfectly good JDL with the new UUID support and get thrown an error when dealing with the generator.

Not sure about that. As the last generator-jhipster release still uses v3.6.12, which doesn't contain this PR.

murdos commented 5 years ago

@murdos I don't see why. If we merge and release, anyone can write a perfectly good JDL with the new UUID support and get thrown an error when dealing with the generator. But I see you've written a test using the UUID type... You're doing dependency testing, here.

So I was wrong, I was thinking that jhipster-core was used more intensively in generator-jhipster. So all validation is only done while interpreting JDL to JSON file, but JSON file is not validated when it's loaded. My bad. That's why I'm able to have passing integration tests with JSON. I however note that this would not have been possible with https://github.com/jhipster/generator-jhipster/issues/9651

I however think that the dependency test is useful, since even if both projects are very linked, they have different release cycle. And it's not uncommon to have releases of generator-jhipster that are missing features because the counterpart of the feature in jhipster-core has not been released. It's also important to have test data in generator-jhipster that are diversified, that's why I added a few UUID.