RamenDR / ramen

Apache License 2.0
74 stars 56 forks source link

e2e: Fix improper use of formatting #1622

Closed alonsofabila-dev closed 1 week ago

alonsofabila-dev commented 4 weeks ago

When using fmt.Errorf(), we don't need to format the message with fmt.Sprintf().

Fixes #1618

alonsofabila-dev commented 4 weeks ago

donde

nirs commented 4 weeks ago

@alonsofabila-dev thanks for the update. If you don't mind I'll fix the commit message myself.

nirs commented 4 weeks ago

@alonsofabila-dev I push another version with some minor changes:

diff --git a/e2e/dractions/retry.go b/e2e/dractions/retry.go
index dddcd38e..cb4a285f 100644
--- a/e2e/dractions/retry.go
+++ b/e2e/dractions/retry.go
@@ -206,7 +206,7 @@ func waitDRPCDeleted(client client.Client, namespace string, name string) error
                }

                if time.Since(startTime) > time.Second*time.Duration(util.Timeout) {
-                       return fmt.Errorf(fmt.Sprintf("drpc %q is not deleted yet before timeout, fail", name))
+                       return fmt.Errorf("drpc %q is not deleted yet before timeout, fail", name)
                }

                time.Sleep(time.Second * time.Duration(util.TimeInterval))
  1. Fix the commit message to be in the standard format
  2. Fix the PR message by copying the commit message
nirs commented 4 weeks ago

@alonsofabila-dev git grep show other instances that you did not fix:

% git grep 'fmt.Errorf(fmt.Sprintf'
deployers/retry.go:                     return fmt.Errorf(fmt.Sprintf("subscription %s status is not %s yet before timeout", name, phase))
deployers/retry.go:                     return fmt.Errorf(fmt.Sprintf("workload %s is not ready yet before timeout of %v",
dractions/retry.go:                     return fmt.Errorf(fmt.Sprintf(
dractions/retry.go:                     return fmt.Errorf(fmt.Sprintf("drpc %s progression is not %s yet before timeout of %v",
alonsofabila-dev commented 4 weeks ago

done

nirs commented 4 weeks ago

@alonsofabila-dev please rebase on main, some of the changes already fixed by another pr merged just now.

nirs commented 3 weeks ago

@alonsofabila-dev I asked to rebase on main, but you merged it - this is not the same. We want to have clean linear history in git without merge commits. The way to update your PR is to use:

git checkout main
git pull upstream main
git checkout your-pr-branch
git rebase main
(fix possible conflicts)
git push -f

This should be done each time you push changes, to make sure your change is tested on top of latest version.

alonsofabila-dev commented 3 weeks ago

sorry for my mistake, I have now done as you mentioned. but keep sayin that there are conflicts, and when i rebase on main says that its up to date, even though i see they are not the same

nirs commented 3 weeks ago

@alonsofabila-dev You rebased, but you had to "pick" only your commit. I clean up and resolved the conflicts.

alonsofabila-dev commented 3 weeks ago

thanks