jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.49k stars 4.02k forks source link

[FEATURE] Remove entity #4372

Closed Dufgui closed 7 years ago

Dufgui commented 7 years ago
Overview of the issue

Remove all files of generated for one entity by the entities sub generator

Motivation for or Use Case

Many people ask to remove entity.

Suggest a Fix

add an option to the entity generator or create a dedicated sub generator

JHipster Version(s)

All

Dufgui commented 7 years ago

PR #4369

Dufgui commented 7 years ago

So to summarize / quote all opinions from the PR chat: Pros:

Cons:

Suggestions:

jdubois commented 7 years ago

There's another "Pro": it might makes upgrades easier

Dufgui commented 7 years ago

My answer: according to the result of this chat:

jdubois commented 7 years ago

OK let's wait a couple of days to have comments from other contributors (and also end-users!), but +1 for me, if so many people ask for this.

Dufgui commented 7 years ago

oki doki ;o). I add a link to here on SO: http://stackoverflow.com/questions/28226336/how-to-delete-an-entity-after-creating-it-using-jhipster/40213511#40213511. In order to have the feedback of end users

cbornet commented 7 years ago

using jhipster --with-entities to clean the old entities could be dangerous for the manual view creaed by the dev. In my case, we change some entity on our model but we have some specific web service, and angular view. So we don't want to remove them when we want to delete an entity. So as for the existing code, it is sensible and must be well tested

That's why I also propose the "jhipster:upgrade" way of doing things which would preserve your manual changes.

cbornet commented 7 years ago

And since you don't need to do any npm install, the process shouldn't be that long.

deepu105 commented 7 years ago

Ok since you moved discussion here ill add my comment here. Sorry for the spam

I agree with the maintenance issue Julien highlighted. We already have a solution to reduce it. Since im working on refactoring the write blocks to use metadata rather than instructions it would be very easy to write or delete files using a simple function so lot less code to maintain. But then I agree that not many would use this.

I also have another idea for the generator on the whole. Ill start a discussion on the mailing list for that first

Thanks & regards, Deepu

On 24 Oct 2016 14:20, "Julien Dubois" notifications@github.com wrote:

There's another "Pro": it might makes upgrades easier

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/4372#issuecomment-255683309, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDlFxdOjXfroHzf8QgatCFB3SoobHvGks5q3HFVgaJpZM4Kehti .

deepu105 commented 7 years ago

Im against additing a new sub generator for this. If we are doing this it should be here as we do create and edit here already so would make sense to do delete here as well. May be we can add the question to ask for delete along with regeneration/edit option when re running the generator

Thanks & regards, Deepu

On 24 Oct 2016 14:22, d4udts@gmail.com wrote:

Ok since you moved discussion here ill add my comment here. Sorry for the spam

I agree with the maintenance issue Julien highlighted. We already have a solution to reduce it. Since im working on refactoring the write blocks to use metadata rather than instructions it would be very easy to write or delete files using a simple function so lot less code to maintain. But then I agree that not many would use this.

I also have another idea for the generator on the whole. Ill start a discussion on the mailing list for that first

Thanks & regards, Deepu

On 24 Oct 2016 14:20, "Julien Dubois" notifications@github.com wrote:

There's another "Pro": it might makes upgrades easier

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/4372#issuecomment-255683309, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDlFxdOjXfroHzf8QgatCFB3SoobHvGks5q3HFVgaJpZM4Kehti .

Dufgui commented 7 years ago

Ok is it possible to be on copy of this discution ? Or maybe I can join the ML ? Just to know how modify this PR please? Just want to know i can help the project. ;o)

cbornet commented 7 years ago

Small script I used as a POC for a git based solution inspired by jhipster:upgrade . It removes the Bar entity.

#!/bin/sh

#Only if you don't use git already
git init
git add -A
git commit -am "Initial" --allow-empty

#Create branch
git checkout --orphan jhipster_remove_entity

#Generate app with the entity
mv src/main/webapp/bower_components bower_components
rm -rf src
yo jhipster --with-entities --force --skip-install
mv bower_components src/main/webapp/bower_components
gulp install

#Commit to branch
git add -A
git commit -am "Generated with Bar entity" --allow-empty

git checkout master
git merge --strategy=ours --no-edit jhipster_remove_entity

git checkout jhipster_remove_entity

#Generate app without the entity
rm .jhipster/Bar.json
mv src/main/webapp/bower_components bower_components
rm -rf src
yo jhipster --with-entities --force --skip-install
mv bower_components src/main/webapp/bower_components
gulp install

#Commit to branch
git add -A
git commit -am "Generated without Bar entity" --allow-empty

#Merge branch into master
git checkout master
git merge jhipster_remove_entity

Pros:

Cons:

cbornet commented 7 years ago

And it's quite fast: about 20s on my slow laptop

Dufgui commented 7 years ago

So you can close my PR, and maybe add this script in the doc for other people, no ?

cbornet commented 7 years ago

@Dufgui It would be better to integrate the script in a subgen. I can do it if we all agree.

Dufgui commented 7 years ago

Ok for me, many thanks, I let other people reply

deepu105 commented 7 years ago

@cbornet ok your script seems better than the manual removal steps by @Dufgui but I have few questions. What happens if there are custom code in entities which are being modified? also I think this would always result in merge conflicts for the user. Also as I said earlier if this is being done it should be part of the entity sub generator as part of this question or as a CLI flag else it will be be like scattered options also I guess all the above items can be done using the gitExec method we have so that its cross platform.

I'lll leave the decision to @jdubois

var prompts = [
        {
            type: 'list',
            name: 'updateEntity',
            message: 'Do you want to update the entity? This will replace the existing files for this entity, all your custom code will be overwritten',
            choices: [
                {
                    value: 'regenerate',
                    name: 'Yes, re generate the entity'
                },
                {
                    value: 'add',
                    name: '[BETA] Yes, add more fields and relationships'
                },
                {
                    value: 'remove',
                    name: '[BETA] Yes, remove fields and relationships'
                },
                {
                    value: 'none',
                    name: 'No, exit'
                }
            ],
            default: 0
        }
    ];
cbornet commented 7 years ago

Yes, if you modified the entity being removed that will conflict. Then the merge must be done with "theirs" strategy to force the suppression.

cbornet commented 7 years ago

As for the git commands, I was thinking of reusing the ones from the upgrade gen (putting them in a utility file). They already use gitExec.

jdubois commented 7 years ago

I like the solution from @cbornet as:

Of course there is still the issue of people changing the code: I expect that most people will change a lot of code. So that's going to be a problem.

But I don't think you can automate everything here: people can do anything with the code, and then if we try to be too smart here we will 1) use a lot of time to think of all the side-issues and 2) have very complex bugs because we can't know everything that people will do with the code

cbornet commented 7 years ago

@jdubois when removing an entity we either:

For the first case, the subgen in PR does a merge with -X theirs strategy. So if your modifications don't conflict they'll just be merged. If they conflict, it means that you modified code in the lines that were added above the needle (eg. you modified the ehcache config for the entity you want to remove) and the removal of those lines is favored. For the second case, you modified a file that's gonna be removed so the subgen forces the remove.

Also note that the subgen will not commit the merge (--no-commit --no-ff) so the user is encouraged to review the modifications himself before committing.

So I think it should work fine for 99.9% of the case and for the 0.1% left, the user can finish the removal by himself.

allquixotic commented 7 years ago

I'm very excited to try this feature - does anyone know if there is an npm command I can run to install a particular version of jhipster that would contain the draft remove entity code? I ask because I'm still in the early design phase of my app where I'm working out the entity model, and I'm frequently having to revert commits, etc. to go back to the "pre-import-jdl" phase of my project, modify the .jdl, and re-import. It'd be nice to have that process automated because so much code is being touched by adding or removing an entity, especially ones with relationships and DTOs and service models.

Thanks!

Dufgui commented 7 years ago

Got clone the repo and branch. The exécute "npm link" inside. The go to your app and use it. If the sub generator is not available you can create a link to your cloned generator in your node module app directory. Le dim. 27 nov. 2016 à 09:34, Sean McNamara notifications@github.com a écrit :

I'm very excited to try this feature - does anyone know if there is an npm command I can run to install a particular version of jhipster that would contain the draft remove entity code? I ask because I'm still in the early design phase of my app where I'm working out the entity model, and I'm frequently having to revert commits, etc. to go back to the "pre- import-jdl" phase of my project, modify the .jdl, and re-import. It'd be nice to have that process automated because so much code is being touched by adding or removing an entity, especially ones with relationships and DTOs and service models.

Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/4372#issuecomment-263108761, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOemzBpjeQPl-zJojlXm6uMtENffyL4ks5rCUB4gaJpZM4Kehti .

jdubois commented 7 years ago

Closing this as it's very complex, and we should focus on JHipster 4 at the moment, see the PR at https://github.com/jhipster/generator-jhipster/pull/4403#issuecomment-272292087 for more info