-
Notifications
You must be signed in to change notification settings - Fork 32
[DSD-9524] Release Platform 1.2.1.0 GA #197
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
Conversation
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
WalkthroughChart and deployment references for keycloak-init were updated: chart version moved from 1.3.0-develop → 1.3.0 and container image repository/tag references changed from mosipqa/...:1.3.x to mosipid/...:1.3.0 across Helm values and deployment scripts. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
helm/keycloak-init/values.yaml (1)
228-228: Consider using boolean values instead of quoted strings for YAML attributes.The static analysis correctly identifies that YAML attributes use string values
"true"and"false"instead of booleantrueandfalse. While functional, using proper boolean values is more idiomatic and reduces potential parsing ambiguity.🔎 Proposed fix to use boolean values
- attributes: {display.on.consent.screen: "false", include.in.token.scope: "true"} + attributes: {display.on.consent.screen: false, include.in.token.scope: true}Apply this pattern to all affected lines (228, 233, 238, 243, 253, 258, 263). Line 248 uses
"true"fordisplay.on.consent.screen, which should also betrue.Note: Verify that Keycloak correctly interprets boolean values before applying this change, as some systems may expect string representations.
Based on static analysis hints.
Also applies to: 233-233, 238-238, 243-243, 248-248, 253-253, 258-258, 263-263
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
helm/keycloak-init/Chart.yamlhelm/keycloak-init/values.yaml
🧰 Additional context used
🪛 GitHub Check: chart-lint-publish / chart-lint-publish
helm/keycloak-init/values.yaml
[warning] 262-262:
262:37 [truthy] truthy value should be one of [false, true]
[warning] 257-257:
257:37 [truthy] truthy value should be one of [false, true]
[warning] 252-252:
252:37 [truthy] truthy value should be one of [false, true]
[warning] 247-247:
247:37 [truthy] truthy value should be one of [false, true]
[warning] 242-242:
242:37 [truthy] truthy value should be one of [false, true]
[warning] 237-237:
237:37 [truthy] truthy value should be one of [false, true]
[warning] 232-232:
232:37 [truthy] truthy value should be one of [false, true]
🔇 Additional comments (1)
helm/keycloak-init/values.yaml (1)
7-10: Image repository and tag changes look correct for GA release.The change from
mosipqa/keycloak-inittomosipid/keycloak-initand the tag pinning from1.3.xto1.3.0are appropriate for a production GA release. Ensure the production image has been built and pushed to the registry.This was already covered by the verification script in Chart.yaml review.
Signed-off-by: Prafulrakhade <[email protected]>
Automated PR for release.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.