Conversation
Signed-off-by: NaYeong,Kim <nayeongkim@megazone.com>
Signed-off-by: NaYeong,Kim <nayeongkim@megazone.com>
Signed-off-by: NaYeong,Kim <nayeongkim@megazone.com>
skdud4659
added
the
self_approved/review
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
There was a problem hiding this comment.
Pull Request Overview
This PR makes minor chore changes including updating localization string text, replacing a placeholder with a numeric input type, and commenting out old temporary asset functionality code.
- Updated the Korean language pack text for anomaly detection notifications.
- Changed the input component in the cost explorer admin page to a number type.
- Commented out sections in alert-manager components related to temporary asset handling.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/language-pack/ko.json | Updated text for the notification rules localization. |
| apps/web/src/services/cost-explorer/pages/admin/AdminAnomalyDetectionConfigurationPage.vue | Changed the input field from displaying a placeholder to enforcing numeric input. |
| apps/web/src/services/alert-manager/v2/components/ServiceDetailTabsSettingsEventRuleFormCard.vue | Commented out the temporary asset validation logic. |
| apps/web/src/services/alert-manager/v2/components/ServiceDetailTabsSettingsEventRuleCard.vue | Commented out code that previously pushed temporary asset options into the result array. |
| apps/web/src/services/alert-manager/v2/components/ServiceDetailTabsSettingsEventRuleActionAsset.vue | Removed unused imports and commented out temporary asset functionality in both script and template sections. |
| // if (state.actions.match_asset?.create_temporary_asset) { | ||
| // const matchAssetValid = state.actions.match_asset.asset_types.length > 0 && state.actions.match_asset.key !== ''; | ||
| // if (!matchAssetValid) return false; | ||
| // } |
There was a problem hiding this comment.
[nitpick] Consider removing or clarifying the commented-out temporary asset validation code if it is no longer needed.
| // if (state.actions.match_asset?.create_temporary_asset) { | |
| // const matchAssetValid = state.actions.match_asset.asset_types.length > 0 && state.actions.match_asset.key !== ''; | |
| // if (!matchAssetValid) return false; | |
| // } | |
| // result[type].push({ | ||
| // label: i18n.t('ALERT_MANAGER.EVENT_RULE.TEMP_ASSET'), | ||
| // name: 'create_temporary_asset', | ||
| // value: matchAssetValue.create_temporary_asset ? i18n.t('ALERT_MANAGER.CREATE') : i18n.t('ALERT_MANAGER.EVENT_RULE.DO_NOT_CREATE'), | ||
| // }); |
There was a problem hiding this comment.
[nitpick] If temporary asset functionality is deprecated, consider permanently removing the commented-out push block to improve code clarity.
| // result[type].push({ | |
| // label: i18n.t('ALERT_MANAGER.EVENT_RULE.TEMP_ASSET'), | |
| // name: 'create_temporary_asset', | |
| // value: matchAssetValue.create_temporary_asset ? i18n.t('ALERT_MANAGER.CREATE') : i18n.t('ALERT_MANAGER.EVENT_RULE.DO_NOT_CREATE'), | |
| // }); |
| key, | ||
| asset_types: assetList?.map((asset) => asset.name), | ||
| create_temporary_asset: tempAsset === 'CREATE', | ||
| // create_temporary_asset: tempAsset === 'CREATE', |
There was a problem hiding this comment.
[nitpick] If the temporary asset functionality is no longer used, consider removing the commented-out assignment to reduce clutter.
| // create_temporary_asset: tempAsset === 'CREATE', |
| </p-field-group> | ||
| </div> | ||
| <div class="field-group flex items-center"> | ||
| <!-- TODO: used when adding temporary asset functionality --> |
There was a problem hiding this comment.
[nitpick] Review the commented HTML block regarding temporary asset functionality to ensure it does not impact the template readability; remove it if it is no longer necessary.
Skip Review (optional)
style,chore,ci,test,docs)Description (optional)
SSIA
Things to Talk About (optional)