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 support to JDL entities at CI scripts. #13204

Merged
merged 7 commits into from
Dec 12, 2020

Conversation

mshima
Copy link
Member

@mshima mshima commented Dec 6, 2020

JDL entities must be executed after copying .yo-rc.json and before generating the project (jhipster --with-entities).

  • Create 11-generate-config.sh and deprecate 11-generate-entities.sh
    11-generate-config.sh copy .yo-rc.json, json entities and generates entities from jdl afterwards.

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

When you are still working on the PR, consider converting it to Draft (bellow reviewers) and adding skip-ci label, you can still see CI build result at your branch.

#-------------------------------------------------------------------------------
# Generate jdl entities
#-------------------------------------------------------------------------------
jhipster --no-insight jdl "$JHI_SAMPLES"/jdl-entities/*.jdl --json-only
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using JDL for generating entities is nice, but I'd prefer another approch:

  • I'd remove this part for sqlfull
  • use another variable: JHI_JDL_NAME
  • use another if block:
if [[ "$JHI_ENTITY" == "jdl" && "$JHI_JDL_NAME" != "" ]]; then
    jhipster --no-insight jdl "$JHI_SAMPLES"/jdl-entities/$JHI_JDL_NAME.jdl --json-only
elif

So it will improve our current CI, don't have any impact for daily builds, blueprints, etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"$JHI_ENTITY" == "jdl" is used for generating the entire project using jdl, it would make this PR useless.

@mshima
Copy link
Member Author

mshima commented Dec 12, 2020

@pascalgrimaud this PR is to simplify entity test cases like https://github.com/jhipster/generator-jhipster/blob/c058e12ba363b815f28ca5b635413f0a42266644/test-integration/samples/jdl-entities/custom-id.jdl.

With current implementation I would have to create 8 different files instead of 1.

@pascalgrimaud
Copy link
Member

So if I understand well, the final idea is to replace all entity json files to JDL, am I correct ?

@mshima
Copy link
Member Author

mshima commented Dec 12, 2020

So if I understand well, the final idea is to replace all entity json files to JDL, am I correct ?

That's an option to consider.
I could generate the files if you prefer to don't mix entity.json and jdl.

@pascalgrimaud
Copy link
Member

pascalgrimaud commented Dec 12, 2020

Yes, I'm fine.
For other blueprints, Daily builds, etc, we just need to keep :

  • the old file 11-generate-entities.sh
  • all json entities

Then, in main generator-jhipster, we can use JDL now. It would be easier for us to reproduce the failures

@pascalgrimaud
Copy link
Member

@mshima : do you need this PR beeing merged to advance on other PR ?
If yes, I'll merge it and we can improve with JDL in another PR

@mshima
Copy link
Member Author

mshima commented Dec 12, 2020

do you need this PR beeing merged to advance on other PR ?

@pascalgrimaud I've create several PRs to reduce the size of PRs and simplify the review and merge process.
It could be merged together or drop test from the other PR.
If this PR is ok I prefer to merge it prior.

@pascalgrimaud pascalgrimaud merged commit 72082c2 into jhipster:main Dec 12, 2020
@pascalgrimaud pascalgrimaud added this to the 7.0.0-beta.0 milestone Dec 18, 2020
@mshima mshima deleted the skip_ci_jdl_entity branch January 18, 2021 17:29
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

2 participants