search

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

Implement the "blueprints" feature #415

Closed MathieuAA closed 4 years ago

MathieuAA commented 4 years ago

Related PR (unfinished): #344 Relevant comment: https://github.com/jhipster/jhipster-core/pull/344#issuecomment-532062824

@pascalgrimaud as discussed two days ago, I believe this feature may deserve a bounty. @murdos as you're the stream lead on blueprints, do you think this is worth making it a priority?

As for this issue, I won't have time to work on this but would be ready to help if need be.

murdos commented 4 years ago

@MathieuAA : yes, I think it should be a priority since blueprints are more and more used and combined. So +1 for adding a bounty.

I think I won't have time to working on it too.

pascalgrimaud commented 4 years ago

I'm so sorry, I forget this ticket. If I remember well, when I got the notification, I was on phone and the labels about "bounty" were not created. That's why I forgot to put one on this... I'm doing this right now

MathieuAA commented 4 years ago

Thank you for your answers. I don't have anything on my plate right now, I'll handle it.

MathieuAA commented 4 years ago

@mshima I'm gonna list every PR that involves blueprints for the JDL here, so as to reorganize bounties. Can you add everything related to the blueprint support for the JDL that you have done? Thanks a lot

mshima commented 4 years ago

@MathieuAA the PR was https://github.com/jhipster/jhipster-core/pull/449, the others changes were minor. The PR was working, I can even create a demo, it should be really easy. Just need to create the json on the generator-jhipster and the blueprint part.

I wasn't doing for the bounty, I did because I was working on https://github.com/jhipster/generator-jhipster/pull/11423 (in the end I am not using some of the changes I've made at jhipster-core anymore =\) and since blueprint is my main interest, I kept working on jhipster-core.

MathieuAA commented 4 years ago

I was talking about the PRs in the generator since the bulk of the changes happened there, what about

About #449, I'm still not convinced validation in JCore is the way to go. The idea behind the issue was to delegate the validation to JCore instead of the caller (the generator/blueprint) doing it, and it makes JCore the "validator" of configs. And it was foolish of me to think that. JCore responsibility is mainly the JDL, but after the merge, a lot of things here will be used in the generator. One the things that I feel we should move out of JCore is the business error checker (for instance, pagination + cassandra configs are forbidden) because it not only concerns JCore but the generator where the same rule is implemented (so two implementations, in two repos).

mshima commented 4 years ago

I was talking about the PRs in the generator since the bulk of the changes happened there, what about

They are typical blueprints fixes, which jdl benefits because it uses generator-jhipster, but this issue is unrelated.

About #449, I'm still not convinced validation in JCore is the way to go.

I agree, but:

The idea behind the issue was to delegate the validation to JCore instead of the caller (the generator/blueprint) doing it, and it makes JCore the "validator" of configs. And it was foolish of me to think that. JCore responsibility is mainly the JDL, but after the merge, a lot of things here will be used in the generator. One the things that I feel we should move out of JCore is the business error checker (for instance, pagination + cassandra configs are forbidden) because it not only concerns JCore but the generator where the same rule is implemented (so two implementations, in two repos).

About this, I am thinking about starting a PR to extract generator-jhipster prompt options to a json (follow up to #449, but useful without #449 which can be shared between jhipster-core, jhipster-ide, jhipster-online, etc). Next step would be extract prompts validation too.

MathieuAA commented 4 years ago

They are typical blueprints fixes, which jdl benefits because it uses generator-jhipster, but this issue is unrelated.

And they might have been done later had you not done them already, which is awesome.

Postponing a (almost done) feature based on only plans (I don't think chevrotain will work with '. .(.*)' syntax).

I don't get it.

About this, I am thinking about starting a PR to extract generator-jhipster prompt options to a json (follow up to #449, but useful without #449 which can be shared between jhipster-core, jhipster-ide, jhipster-online, etc). Next step would be extract prompts validation too.

That's a good idea. What are your plans for that?

mshima commented 4 years ago

I don't get it.

chevrotain needs a minimum definitions to parse. I have doubts we can parse the jdl without adding, for example, that blueprints options have the format ${key} ${array}, baseName ${key} ${value}, etc. So some kind of validation is required.

About this, I am thinking about starting a PR to extract generator-jhipster prompt options to a json (follow up to #449, but useful without #449 which can be shared between jhipster-core, jhipster-ide, jhipster-online, etc). Next step would be extract prompts validation too.

That's a good idea. What are your plans for that?

Create a json at generator-jhipster/shared/options.json. The options.json should look something like the json I've posted at https://github.com/jhipster/jhipster-core/pull/449#issuecomment-612435233.

Validation is more complicated, it should be modular, to be injected into the prompt and used at app generator configuring step. But the structure would be too complicated to allow adding support to not javascript projects.

MathieuAA commented 4 years ago

I have doubts we can parse the jdl without adding, for example, that blueprints options have the format ${key} ${array}, baseName ${key} ${value}, etc. So some kind of validation is required.

If a use case is provided, this can be discussed and/or implemented.

Create a json at generator-jhipster/shared/options.json. The options.json should look something like the json I've posted at #449 (comment). Validation is more complicated, it should be modular, to be injected into the prompt and used at app generator configuring step. But the structure would be too complicated to allow adding support to not javascript projects.

There's jhipster-base, maybe. This may be a good idea to involve Serano for this.

mshima commented 4 years ago

I have doubts we can parse the jdl without adding, for example, that blueprints options have the format ${key} ${array}, baseName ${key} ${value}, etc. So some kind of validation is required.

If a use case is provided, this can be discussed and/or implemented.

For blueprints option support for example, can you add support to it without add the specific blueprints option to the parser? Thats is the same thing that adding any option the blueprint wants to add. If you can, so I am wrong.

Create a json at generator-jhipster/shared/options.json. The options.json should look something like the json I've posted at #449 (comment). Validation is more complicated, it should be modular, to be injected into the prompt and used at app generator configuring step. But the structure would be too complicated to allow adding support to not javascript projects.

There's jhipster-base, maybe. This may be a good idea to involve Serano for this.

We should keep at generator-jhipster for now. A basic structure at generator-jhipster should be the priority right now. V7 windows will open soon, we don't need to keep it stable for long.

mshima commented 4 years ago

@MathieuAA see: https://github.com/mshima/generator-jhipster/commit/b08c166731f63271cdf885814999e180141a96cc