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

Extend JHipster's Entity Sub-Generator #109

Merged
merged 2 commits into from
Aug 16, 2019

Conversation

mraible
Copy link
Collaborator

@mraible mraible commented Jun 25, 2019

No description provided.

@mraible
Copy link
Collaborator Author

mraible commented Jun 25, 2019

@pvliss I tried this tonight and it seems to fail because it wants to load the JHipster config from the current directory.

➜  ionic4j git:(master) ✗ yo jhipster-ionic:entity Profil
WARNING! Deprecated: JHipster seems to be invoked using Yeoman command. Please use the JHipster CLI. Run jhipster <command> instead of yo jhipster:<command>
events.js:174
      throw er; // Unhandled 'error' event
      ^

TypeError: Cannot read property 'blueprint' of undefined
    at Object.getAllJhipsterConfig (/Users/mraible/dev/jhipster/generator-jhipster-ionic/node_modules/generator-jhipster/generators/utils.js:424:27)

The current directory is my Ionic app and it doesn't have any JHipster config. Is it possible to configure it to look for the file based on the contents of .yo-rc.json? For example:

{
  "generator-jhipster-ionic": {
    "promptValues": {
      "backendPath": "../backend"
    }
  }
}

@pvliss
Copy link

pvliss commented Jun 26, 2019

@mraible I actually only tried running yo jhipster-ionic from the parent dir, re-added the backend path at the prompt and it worked same as with the currently released version. Did not test any further. Will check though and see if that can be overridden somehow

@pvliss
Copy link

pvliss commented Jun 26, 2019

Hmmm. So it seems to me that ionic4j does not automatically pick up the entities from the backend dir and I can not find any compose functionality from the app to the entity sub-gen, so basically my test was worthless. I was under the impression that it did automatically pick up the entities from backend but did not really check either tbh. Sorry for misleading you and will check further to see if it can actually be used that way. So currently you are adding the entities one by one?

@mraible
Copy link
Collaborator Author

mraible commented Jun 26, 2019 via email

@pvliss
Copy link

pvliss commented Jun 26, 2019

OK, so I think I found a workaround by adding a separate .jhipster-ionic.json within the generated mobile app directory that can be used by the entity sug-gen to later define the root dir of the .yo-rc.json file to load. Of course some modifications are needed in the jhipster generator as well in order to allow the config from another dir than the cwd. Have a look here: pvliss/generator-jhipster@bb77fde

Also tried using yeoman's .yo-rc.json from within the app gen but could not get it to play properly due to:

  1. The file is being saved in the parent dir which most likely is not what you want
  2. The file is picked up by yeoman when running the entity gen and changes target dir to the parent dir

@mraible
Copy link
Collaborator Author

mraible commented Jun 26, 2019

@pvliss Wow! Nice work. 👍

I like it. Any downsides you see?

/cc @ruddell might be interested in this for Ignite JHipster.

@mraible mraible closed this Jun 26, 2019
@mraible mraible reopened this Jun 26, 2019
@pvliss
Copy link

pvliss commented Jun 26, 2019

I like it.

Did you give it a spin?

Any downsides you see?

The downside is that you are more dependent on the jhipster entity sub-gen but I would not say that is a big problem in this particular case 😄 .You will always be to find a way around any upcoming issues in the future. The upside of course is much less code to maintain and test.

BTW, the tests seem to be failing due to git checkout failure:

fatal: 'origin/test-extend-entity-subgen' is not a commit and a branch 'test-extend-entity-subgen' cannot be created from it

@mraible
Copy link
Collaborator Author

mraible commented Jun 27, 2019

@pvliss There are issues with how I have Travis currently configured. It expects PRs to have branches in the main repo instead of contributor's forks.

See .travis.yml#L50 and 01-install-jhipster-stack.sh#L37. I don't see any Travis convenience variables we can use for this value.

@mraible
Copy link
Collaborator Author

mraible commented Jun 27, 2019

@pvliss Can you please rebase? Also, I prefer 2 spaces for JavaScript, so please use 2 spaces (not 4) in your branch.

@pvliss
Copy link

pvliss commented Jun 27, 2019

@mraible Would you mind if I create a new proper PR to clean commits, restore formatting and also allow for the CI tests to be executed properly?

@mraible
Copy link
Collaborator Author

mraible commented Jun 27, 2019

@pvliss That's fine.

@pvliss pvliss force-pushed the test-extend-entity-subgen branch from 78aee86 to af30334 Compare June 27, 2019 09:31
@pvliss pvliss mentioned this pull request Jun 27, 2019
@pvliss pvliss force-pushed the test-extend-entity-subgen branch from fdb1a68 to af30334 Compare June 27, 2019 09:51
@pvliss
Copy link

pvliss commented Jun 27, 2019

@mraible Would you like me to rebase again?

@pvliss pvliss force-pushed the test-extend-entity-subgen branch from af30334 to 0a4db62 Compare June 27, 2019 10:01
@pvliss
Copy link

pvliss commented Jun 27, 2019

Just rebased. Let wait and see

@mraible
Copy link
Collaborator Author

mraible commented Aug 5, 2019

@pvliss Can you please rebase this PR to use the latest master branch? Thanks!

@pvliss
Copy link

pvliss commented Aug 8, 2019

@mraible Rebase is done. I did not have the time to test manually though and CI tests seem to fail at e2e tests but not sure if related to the changes of this PR. Can you please check?

@mraible mraible merged commit fbcadb1 into jhipster:master Aug 16, 2019
@mraible
Copy link
Collaborator Author

mraible commented Aug 16, 2019

Thanks for making Ionic for JHipster drastically easier to maintain @pvliss!

@mraible
Copy link
Collaborator Author

mraible commented Aug 17, 2019

@pvliss Do you think we can do something similar to this to support an import-jdl command? #29

@pvliss pvliss deleted the test-extend-entity-subgen branch August 17, 2019 18:25
@pvliss
Copy link

pvliss commented Aug 17, 2019

Thanks for making Ionic for JHipster drastically easier to maintain @pvliss!

Glad to be of help!!!

@pvliss Do you think we can do something similar to this to support an import-jdl command? #29

Will try to find sometime and look into it. I suppose you would prefer ionic4jhipster to remain a module instead of converting it to a blueprint, right, as with blueprint it would be much simpler?

@mraible
Copy link
Collaborator Author

mraible commented Aug 18, 2019 via email

@mraible mraible mentioned this pull request Aug 24, 2019
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.

2 participants