kubesphere-sigs / ks

ks is a tool that makes it be easy to work with KubeSphere.
MIT License
22 stars 16 forks source link

Fix wrong check in function getCompletionTimeFromObject #237

Closed JohnNiang closed 2 years ago

JohnNiang commented 2 years ago

What this PR dose?

Fix wrong check in function getCompletionTimeFromObject, please see:

https://github.com/kubesphere-sigs/ks/blob/88e11d9f6d1769bd56ac56c9bf9da3bd580b9a32/kubectl-plugin/pipeline/gc.go#L131

If the ok is false and err is nil, the return value will be time.Time{}, nil, it's unexpected. Meanwhile, I've add related test cases for that scenario.

BTW, this will make function ascOrderWithCompletionTime behavior unexpectedly. So our latest PipelineRuns will be accidentally deleted.

Why we need it?

Please see #236.

Which issue dose this PR fix?

Fix #236.

/kind bug

codecov[bot] commented 2 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@88e11d9). Click here to learn what that means. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #237   +/-   ##
=========================================
  Coverage          ?   10.94%           
=========================================
  Files             ?       33           
  Lines             ?     1453           
  Branches          ?        0           
=========================================
  Hits              ?      159           
  Misses            ?     1283           
  Partials          ?       11           
Flag Coverage Δ
unittests 10.94% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 88e11d9...5006b84. Read the comment docs.

LinuxSuRen commented 2 years ago

goreleaser can build binary files and images like this:

➜  ks git:(bug/incorrect-gc-strategy) goreleaser --snapshot --rm-dist
.......
      • building docker image     image=surenpi/ks:latest
.....