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 module list to the JModuleProperties #9390

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gzsombor
Copy link
Member

So during module application, a module can detect, what other modules are present. This could be used for optional dependencies

So during module application, a module can detect, what other modules are present.
This could be used for optional dependencies
@DamnClin
Copy link
Collaborator

I don't think this belong to properties, I don't see the responsibility here.

If a module needs to known the applied module I think we should go for something to inject in the module factory, a lite version of ProjectsRepository in the module context seems totally legit to do that.

What do you think?

@gzsombor
Copy link
Member Author

I don't think this belong to properties, I don't see the responsibility here.

If a module needs to known the applied module I think we should go for something to inject in the module factory, a lite version of ProjectsRepository in the module context seems totally legit to do that.

What do you think?

How these module names differs from the "server port", "identation level", "java version", etc ? IMHO they are all properties of the module being generated.
You can put the code, which collects all the already applied modules into a separate service, but you still need to get the list of 'newly applied' modules from the REST endpoint - and that you need to put into JHipsterModuleProperties. We can create a separate class to hold these values - 'PluginDependencies', if you prefer.

@DamnClin
Copy link
Collaborator

I don't think this belong to properties, I don't see the responsibility here.
If a module needs to known the applied module I think we should go for something to inject in the module factory, a lite version of ProjectsRepository in the module context seems totally legit to do that.
What do you think?

How these module names differs from the "server port", "identation level", "java version", etc ? IMHO they are all properties of the module being generated. You can put the code, which collects all the already applied modules into a separate service, but you still need to get the list of 'newly applied' modules from the REST endpoint - and that you need to put into JHipsterModuleProperties. We can create a separate class to hold these values - 'PluginDependencies', if you prefer.

I don't see a case where you would need module that are not yet applied since the modules are applied in dependencies order (otherwise it just won't work).

I'm not a fan of adding a feature without using it in a business case since it's really hard to see if that feet a need. Can you use the feature to get a better grasp of what you have in mind?

@gzsombor
Copy link
Member Author

gzsombor commented May 8, 2024

I try to create a module X, that doesn't depend on module Y, but if Y is present, it needs to do things differently. My example module just wants to generate a mise.toml file for Mise, and needs to know, if Maven/Gradle/Npm is required or not (Mise is basically an advanced version of the Maven/Gradle/Npm wrappers). Similarly, the github/etc-workflow generation could be altered later, if 'Mise' present, no need to deal with all the wrappers and installation, just call Mise to setup the environment.
It's not yet a perfect solution, because it could happen, that someone first applies the Mise module, and later - in a different http request - applies the Maven one, but if it happens in the same http request, it works correctly.

@DamnClin
Copy link
Collaborator

DamnClin commented May 8, 2024

I try to create a module X, that doesn't depend on module Y, but if Y is present, it needs to do things differently. My example module just wants to generate a mise.toml file for Mise, and needs to know, if Maven/Gradle/Npm is required or not (Mise is basically an advanced version of the Maven/Gradle/Npm wrappers). Similarly, the github/etc-workflow generation could be altered later, if 'Mise' present, no need to deal with all the wrappers and installation, just call Mise to setup the environment. It's not yet a perfect solution, because it could happen, that someone first applies the Mise module, and later - in a different http request - applies the Maven one, but if it happens in the same http request, it works correctly.

From what I understand reading the applied modules will provide a better solution since:

  • It will work even in different http request;
  • It will keep a clear separation of concerns;
  • It won't break the existing architecture.

I don't see the point in doing it that way, what are the pros?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants