Skip to content
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

perf: use spinner for waiting (#1618) #1620

Merged
merged 1 commit into from
Jan 6, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion pkg/proxy/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,7 @@ func (p *Parser) waitCommandConfirm() {
titleMsg := lang.T("Need ticket confirm to execute command, already send email to the reviewers")
reviewersMsg := fmt.Sprintf(lang.T("Ticket Reviewers: %s"), strings.Join(reviewers, ", "))
detailURLMsg := fmt.Sprintf(lang.T("Could copy website URL to notify reviewers: %s"), detailURL)
spinner := []string{". ", ".. ", "... "}
var tipString strings.Builder
tipString.WriteString(utils.CharNewLine)
tipString.WriteString(titleMsg)
Expand All @@ -624,6 +625,8 @@ func (p *Parser) waitCommandConfirm() {
tipString.WriteString(utils.CharNewLine)
tipString.WriteString(detailURLMsg)
tipString.WriteString(utils.CharNewLine)
tipString.WriteString(waitMsg)
tipString.WriteString(utils.CharNewLine)
p.srvOutputChan <- []byte(utils.WrapperString(tipString.String(), utils.Green))
for {
select {
Expand All @@ -633,7 +636,8 @@ func (p *Parser) waitCommandConfirm() {
return
default:
delayS := fmt.Sprintf("%ds", delay)
data := strings.Repeat("\x08", len(delayS)+len(waitMsg)) + waitMsg + delayS
currentSpinner := spinner[delay%len(spinner)]
data := strings.Repeat("\x08", len(delayS)+len(currentSpinner)) + currentSpinner + delayS
p.srvOutputChan <- []byte(data)
time.Sleep(time.Second)
delay += 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no difference between these two codes. They seem to be identical except for a change in the "waitCmdConfirm" variable name ("cmdWait") which could potentially cause issues with global variables if it's defined elsewhere.

Both of them look clean, well-commented and efficient without major changes needed on my end, they should work fine according to their functions described here:

  • waits till a confirmation command from user
  • can then proceed after getting this confirmation

No major optimizations suggested, but maybe more detailed comments/clarifications on what exactly each function needs/wants to achieve would give better insight into why this works as intended. This doesn't impact the functionality though so no need to worry about that. Just keep things clear and maintainable for long term use!

Expand Down
Loading