-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
use constants in generator docker #14563
Conversation
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.
Would be nice if you limit PRs to ~20 files.
It will be a lot easier.
7f20753
to
2feac12
Compare
@mshima better now? FYI, I removed the elasticSearchFalse in favour of testing elasticSearch as it is set to |
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 entity* generators needs more work at derived properties due to Microservice support.
Some config is loaded from the gateway, others from Microservice.
Others seems ok.
IMO you should remove derived properties from entities at this PR, just use constants.
Microservice support is really complicated.
fb3a495
to
3a76bd3
Compare
This entity-client generator's code is crazy: so much nifty details everywhere... |
@mshima or @pascalgrimaud can you take a look and maybe quick (as any modification on the angular entity part will lead to a conflict)? I know it's tough and I'm sorry for that, but I think I found the elegant way to achieve it and now templates are way more clean. I guess that the overall refacto will need two or three more full refinement but as it will only be at the js level after the first pass we'll be able to make something way more scalable and modular (I can't wait to split up that 2.6 thousand-line BaseGenerator class :-p) |
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.
@Tcharl last commit is not ok, or it fixes bugs, not sure. The generated code is not identical anymore.
While testing, I've realized that micro service support was already buggy for entity due to a racing condition. We will fix this later.
Revert last commit and merge.
Create a new PR with the last commit.
Sorry @mshima but I don't understand your remark: should I only accept your change and that's it? |
Yes, it stopped recalculating jhiPrefixDashed.
See the builds bellow. You will see the diff:
|
Co-authored-by: Marcelo Shima <[email protected]>
I got the one you mentioned :-) and thank you for the TIPs!. It was a bug on my side (I used Temporal which included ZonedDateTime, LocalDate and Instant instead of Timed, which only includes ZonedDateTime and Instant). Let's rebuild it and see if things are always modified: fingers crossed! Anyway, the review is still welcome: I'll hope I don't have included other side effect |
So, good to go? Ready for the next round (I promise, it will be less than 20 files this time ;-)) |
Go ahead @Tcharl ! |
contributes to generator understanding
Use constants in generator docker
Please make sure the below checklist is followed for Pull Requests.
When you are still working on the PR, consider converting it to Draft (bellow reviewers) and adding
skip-ci
label, you can still see CI build result at your branch.