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 optional rank at modules domain #11724

Merged

Conversation

renanfranca
Copy link
Contributor

@renanfranca renanfranca commented Jan 14, 2025

@renanfranca
Copy link
Contributor Author

Thank you for the reviews. I will work on this tomorrow ✌️😀

@renanfranca renanfranca force-pushed the 10934-add-optional-rank-at-modules branch from 24f4748 to 1d8cdb0 Compare January 22, 2025 14:30
@pascalgrimaud
Copy link
Member

@renanfranca : can you squash all your commits plz?

@renanfranca renanfranca force-pushed the 10934-add-optional-rank-at-modules branch from 758b9c5 to df3f2aa Compare January 22, 2025 21:47
@renanfranca
Copy link
Contributor Author

@renanfranca : can you squash all your commits plz?

@pascalgrimaud : sure, it is done 👍

package tech.jhipster.lite.module.domain.resource;

public enum JHipsterModuleRank {
/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure we should keep all these comments, as we'll need to update them, as soon as there are some change.

For example: as soon as kipe-authorization change his RANK, the comment here won't be correct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pascalgrimaud: I agree that the comments weren't timeless. I improved the comment descriptions. I am trying to keep the comments because it is hard to understand rank usage, and I think the enum is the best place to explain it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is hard to understand rank usage

I'm also a bit sceptical about these rank usages that are very subjective, so I'm curious to see how this will turn out :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my understanding, the ranks are curated by the jhipster lite developers to guide those who are using the tool for the first time. Let's give a try ;)

Copy link
Member

@pascalgrimaud pascalgrimaud Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will not impact the final user, as we'll always display by default "ALL RANK".

Then, for example, the user can select the RANK_S to display only the most used modules. I don't think he/she'll be unhappy because his prefered module is not displayed in RANK_S, because his module will always be in "ALL RANK", so I'm fine with this

Copy link
Contributor Author

@renanfranca renanfranca Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pascalgrimaud : when you have the time, please, let me know if I can keep these improved comments. Thanks :)

@renanfranca renanfranca force-pushed the 10934-add-optional-rank-at-modules branch from df3f2aa to ddb7ffb Compare January 23, 2025 11:43
@pascalgrimaud pascalgrimaud merged commit f6b47c2 into jhipster:main Jan 25, 2025
37 checks passed
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.

3 participants