feat: use deepin-immutable-ctrl to wrap and call locale-gen#136
Conversation
Reviewer's GuideThis PR adds conditional wrapping of locale-gen with deepin-immutable-ctl to avoid permission issues on immutable systems, falling back to direct execution when the wrapper is unavailable. Sequence diagram for locale generation with deepin-immutable-ctl fallbacksequenceDiagram
participant Helper
participant dutils
participant deepin-immutable-ctl
participant locale-gen
Helper->>dutils: IsFileExist(deepin-immutable-ctl)
alt deepin-immutable-ctl exists
Helper->>deepin-immutable-ctl: exec locale-gen
deepin-immutable-ctl->>locale-gen: run
locale-gen-->>deepin-immutable-ctl: result
deepin-immutable-ctl-->>Helper: output
else deepin-immutable-ctl not found
Helper->>locale-gen: run directly
locale-gen-->>Helper: result
end
Class diagram for updated Helper locale generation methodsclassDiagram
class Helper {
+doGenLocale() error
+doGenLocaleWithParam(locale string) error
}
class dutils {
+IsFileExist(path string) bool
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @mhduiy - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `locale-helper/main.go:94` </location>
<code_context>
}
func (h *Helper) doGenLocale() error {
- return exec.Command(localeGenBin).Run()
+ if !dutils.IsFileExist(deepinImmutableCtlBin) {
</code_context>
<issue_to_address>
Consider extracting the duplicated logic in both methods into a single helper function to simplify the code.
You can collapse the nearly‐identical logic in both methods into a single helper. For example:
```go
// new helper extracted from doGenLocale + doGenLocaleWithParam
func (h *Helper) execLocaleCmd(args ...string) error {
if !dutils.IsFileExist(deepinImmutableCtlBin) {
logger.Warning("deepin-immutable-ctl not found, use locale-gen directly")
return exec.Command(localeGenBin, args...).Run()
}
// Use “--” to separate flags from the command itself
ctlArgs := append([]string{"admin", "exec", "--", localeGenBin}, args...)
output, err := exec.Command(deepinImmutableCtlBin, ctlArgs...).CombinedOutput()
if err != nil {
logger.Warning(
"deepin-immutable-ctl exec locale-gen failed,",
"err:", err,
"output:", string(output),
)
}
return err
}
```
Then simplify your two API methods to:
```go
func (h *Helper) doGenLocale() error {
return h.execLocaleCmd()
}
func (h *Helper) doGenLocaleWithParam(locale string) error {
return h.execLocaleCmd(locale)
}
```
This removes the duplication, keeps the exact same behavior, and flattens the control flow.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -90,10 +92,32 @@ func (h *Helper) canQuit() bool { | |||
| } | |||
|
|
|||
| func (h *Helper) doGenLocale() error { | |||
There was a problem hiding this comment.
issue (complexity): Consider extracting the duplicated logic in both methods into a single helper function to simplify the code.
You can collapse the nearly‐identical logic in both methods into a single helper. For example:
// new helper extracted from doGenLocale + doGenLocaleWithParam
func (h *Helper) execLocaleCmd(args ...string) error {
if !dutils.IsFileExist(deepinImmutableCtlBin) {
logger.Warning("deepin-immutable-ctl not found, use locale-gen directly")
return exec.Command(localeGenBin, args...).Run()
}
// Use “--” to separate flags from the command itself
ctlArgs := append([]string{"admin", "exec", "--", localeGenBin}, args...)
output, err := exec.Command(deepinImmutableCtlBin, ctlArgs...).CombinedOutput()
if err != nil {
logger.Warning(
"deepin-immutable-ctl exec locale-gen failed,",
"err:", err,
"output:", string(output),
)
}
return err
}Then simplify your two API methods to:
func (h *Helper) doGenLocale() error {
return h.execLocaleCmd()
}
func (h *Helper) doGenLocaleWithParam(locale string) error {
return h.execLocaleCmd(locale)
}This removes the duplication, keeps the exact same behavior, and flattens the control flow.
Avoiding permission issues caused by immutable systems Log: use `deepin-immutable-ctrl` to wrap and call `locale-gen`
deepin pr auto review代码审查意见：
总体来说，代码中存在一些重复、错误处理不足、日志记录不明确、安全性问题以及临时解决方案等问题，建议进行相应的修改和优化。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, KT-lcz, mhduiy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
5 participants
Avoiding permission issues caused by immutable systems
Log: use
deepin-immutable-ctrlto wrap and calllocale-genSummary by Sourcery
Wrap calls to locale-gen with deepin-immutable-ctl on immutable systems to avoid permission issues, falling back to direct execution when the wrapper is unavailable.
Bug Fixes:
Enhancements: