-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Finalize unambiguous priorities api changes. #19978
Conversation
PRIORITY_NAMES.PREPARING, | ||
CONFIGURING_EACH_ENTITY, | ||
LOADING_ENTITIES, | ||
PREPARING_EACH_ENTITY, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The EACH term is a bit confusing, it could mean that it will modify EACH Entities ^^.
Removing the EACH looks to be more meaningful (meaning that we are focusing on one specific entity).
Am I wrong with the meaning? If it really means EACH, I would go for PREPARING_ENTITIES.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can rename, but let's do it in another PR.
@@ -0,0 +1,138 @@ | |||
const { PRIORITY_PREFIX, QUEUE_PREFIX, PRIORITY_NAMES, QUEUES } = require('../base/priorities.cjs'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it worth it to create a base-platform/priorities.cjs
(a bunch of application), a base-entity/priorities.cjs
and maybe a base-field/priorities.cjs
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it worth it to create a
base-platform/priorities.cjs
(a bunch of application).
Yes, I think we should call it base-workspaces
.
I will create a feature request.
a
base-entity/priorities.cjs
and maybe abase-field/priorities.cjs
?
I don't think so.
But I think we should implement a base-incremental-application for entity changes like liquibase and maybe entity removal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some users are complaining that generation is all or nothing. Splitting metamodels (and models) could ease to generate only entities and fields that have been updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -508,7 +508,7 @@ module.exports = class JHipsterServerGenerator extends BaseApplicationGenerator | |||
} | |||
|
|||
get [BaseApplicationGenerator.CONFIGURING_EACH_ENTITY]() { | |||
return this.asPromptingTaskGroup(this.delegateToBlueprint ? {} : this.configuringEachEntity); | |||
return this.asConfiguringEachEntityTaskGroup(this.delegateToBlueprint ? {} : this.configuringEachEntity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idem: asConfiguringEntityTaskGroup
Reviewed
|
@Tcharl can this PR be merged? |
I took a brief look and it seems like most phases are suffixed with 'ing', which makes me think they are called at the beginning of a phase. The only one that looks weird is "END", which might make sense if it's called at the very end. |
generator-base-blueprint and generator-base are in the inheritance tree, but should be dropped for v8.
Closes #17132
Please make sure the below checklist is followed for Pull Requests.
When you are still working on the PR, consider converting it to Draft (below reviewers) and adding
skip-ci
label, you can still see CI build result at your branch.