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

Modules landscape #3022

Merged
merged 1 commit into from Aug 12, 2022
Merged

Modules landscape #3022

merged 1 commit into from Aug 12, 2022

Conversation

DamnClin
Copy link
Collaborator

@DamnClin DamnClin commented Aug 11, 2022

Close #2996
Related to #1666

This is a first version, still not really usable as is, here is the beginning of the tree:
level1-2

This first part is OK but then comes a problem when we have modules depending on multiple levels, here is one of the worst part (bottom of levels 4 to 6):
level4-6

Easy problems to tackle next:

Harder problems:

  • Some dependencies are probably not good. The messing lines may indicate that, we probably need to rethink some of the dependencies;
  • Reaching something really readable...
  • Were do we put the form with all properties for application of multiple modules?
  • And probably lot of other but I think this is an important first step!

@codecov
Copy link

codecov bot commented Aug 11, 2022

Codecov Report

Merging #3022 (57d3506) into main (fd8f131) will not change coverage.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                main     #3022    +/-   ##
============================================
  Coverage     100.00%   100.00%            
- Complexity      1979      2073    +94     
============================================
  Files            467       506    +39     
  Lines           7975      8448   +473     
  Branches         153       161     +8     
============================================
+ Hits            7975      8448   +473     
Impacted Files Coverage Δ
...ool/maven/application/MavenApplicationService.java 100.00% <ø> (ø)
...tor/typescript/domain/TypescriptModuleFactory.java 100.00% <ø> (ø)
...odule/domain/resource/DuplicatedSlugException.java 100.00% <ø> (ø)
...n/resource/JHipsterModulePropertiesDefinition.java 100.00% <ø> (ø)
...ain/resource/JHipsterModulePropertyDefinition.java 100.00% <ø> (ø)
...e/primary/JHipsterModuleApplicationController.java 100.00% <ø> (ø)
...frastructure/primary/JHipsterModuleController.java 100.00% <ø> (ø)
.../JHipsterModulePropertiesDefinitionController.java 100.00% <ø> (ø)
...e/primary/JHipsterModulesLegacyHandlerMapping.java 100.00% <ø> (ø)
...JHipsterModulesPatchApplicationHandlerMapping.java 100.00% <ø> (ø)
... and 122 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@DamnClin DamnClin marked this pull request as ready for review August 11, 2022 16:44
@DamnClin
Copy link
Collaborator Author

DamnClin commented Aug 11, 2022

An idea may be to merge all dependencies from a feature to something so we can have less lines

In the current landscape this removes 17 connections, it require some effort but it's probably worth it

@DamnClin DamnClin marked this pull request as draft August 11, 2022 16:49
@DamnClin
Copy link
Collaborator Author

DamnClin commented Aug 11, 2022

Back to draft: just saw that resizes when scrolled send the lines very far, need to fix that :)

-> Fixed

@DamnClin DamnClin marked this pull request as ready for review August 11, 2022 17:37
@DamnClin DamnClin force-pushed the modules-landscape branch 2 times, most recently from 14611b9 to 2bbc84b Compare August 11, 2022 18:05
@DamnClin
Copy link
Collaborator Author

Overnight though:

  • The application form can be a fixed block top left;
  • We should give lots of hints during selection:
    • Cleary mark non selectable modules
    • Change line shape (full vs dotted) for links to selected modules VS not selected modules
    • Change line and box color for selected module (use primary)
    • Animate linked elements when going over a not selected module (something like a color change)

But all this will be in dedicated PR since this one is already huge and we first need to reorganize some modules

@DamnClin DamnClin merged commit 337cc0e into jhipster:main Aug 12, 2022
@DamnClin DamnClin deleted the modules-landscape branch August 12, 2022 06:59
@hdurix
Copy link
Member

hdurix commented Aug 12, 2022

Good idea to split PRs 😄

I've just read this PR, seems good. Nice work!!

@DamnClin
Copy link
Collaborator Author

Good idea to split PRs
Yep... sorry for this one...

I've just read this PR, seems good. Nice work!!
Still not really usable but I think this is an interesting step to have a really usable JHlite

@hdurix
Copy link
Member

hdurix commented Aug 12, 2022

Do you think it could be relevant to have only 1 line from a module to a feature? Instead of having 1 line for each module of the feature?

image

Here a line from springboot to jpa-persistence.

Do you think that in a feature, modules could have different dependencies?

@DamnClin
Copy link
Collaborator Author

Not for all cases (because sometimes all modules in the same feature don't share the same dependencies) but this is this comment:

An idea may be to merge all dependencies from a feature to something so we can have less lines

In the current landscape this removes 17 connections, it require some effort but it's probably worth it

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.

Display module landscape
2 participants