use derived constants in kubernetes base #14767
Conversation
Tcharl
changed the title
|
I'm really not sure about this one, but hope the approach is right |
|
I really suck at restructuring, and I don't really understand how to achieve that flattening of internal methods from another class. Any js blackbelt to help? |
|
@Tcharl last commit you changed |
|
I am thinking about adding snapshots test for deployment generators. |
|
Thank you vers much for your time @mshima , I'll one day become a js blackbelt because of your support! Fixing the issue now, what about the remaining change? |
|
Triple checked: is it ok to go? |
|
Hi everyone, Back to that constants stuff ;-). Thank you very much one more time for the snapshot testing @mshima , it's awesome for troubleshooting! I think the new snapshots are only fixing existing bugs, can someone take a look please? @deepu105 , I think that you're the one able to tell if the new version generated for the 'kubernetes helm: gateway, mysql, psql, mongodb, mariadb microservices' test is better than the old one. |
|
I'll try it out when i hv time |
|
Glad it worked. |
|
Yes, it is totally voluntary: I wanted to wait for the review so that buddies will know what changed, then I'll update the snapshot if everything is validated |
|
Much more confident with this round :-) |
| @@ -484,16 +486,19 @@ which is free for the first 30 days`); | |||
| return this._default(); | |||
| } | |||
|
|
|||
| _loadPlatformConfig(config = _.defaults({}, this.jhipsterConfig, defaultConfig), dest = this) { | |||
| super.loadPlatformConfig(config, dest); | |||
| _computeDerivedConfig(config = _.defaults({}, this.jhipsterConfig, defaultConfig), dest = this) { | |||
There was a problem hiding this comment.
Move to after priorities, it's not a priority.
Or drop the method and put in the task.
| _loadPlatformConfig(config = _.defaults({}, this.jhipsterConfig, defaultConfig), dest = this) { | ||
| super.loadPlatformConfig(config, dest); | ||
| dest.cicdIntegrationsSnyk = config.cicdIntegrations || []; | ||
| _loadCiCdConfig(config = _.defaults({}, this.jhipsterConfig, defaultConfig), dest = this) { |
There was a problem hiding this comment.
Better move to after priorities to don't mix priority and api.
|
Pending for too long: merging it, and pursuing the adventure after looking at the 'two phase generation' PR |
contributes to #14235
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-cilabel, you can still see CI build result at your branch.