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

*: tidy code && prealloc some slice for performance #13468

Merged
merged 11 commits into from
Jan 2, 2020

Conversation

xiekeyi98
Copy link
Contributor

@xiekeyi98 xiekeyi98 commented Nov 14, 2019

What problem does this PR solve?

  • Tidy code
  • Prealloc some slice for performance.

Tests

  • Unit test
  • Integration test

This change is Reviewable

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Nov 14, 2019
@@ -140,11 +140,11 @@ LOOP:
break LOOP
default:
if strings.HasPrefix(s, "--error") {
t.expectedErrs = strings.Split(strings.TrimSpace(strings.TrimLeft(s, "--error")), ",")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

// TrimLeft returns a slice of the string s with all leading
// Unicode code points contained in cutset removed.
//
// To remove a prefix, use TrimPrefix instead.

As the context (HasPrefix) , I think we should use TrimPrefix

ddl/db_test.go Outdated
@@ -3606,7 +3606,7 @@ func (s *testDBSuite2) TestTablesLockDelayClean(c *C) {
tk.Se.Close()
wg.Done()
}()
time.Sleep(50)
time.Sleep(50 * time.Nanosecond)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

time.Sleep(50) is puzzled, we should add the unit.

@@ -225,7 +225,7 @@ func TestSyncerSimple(t *testing.T) {
}

func isTimeoutError(err error) bool {
if terror.ErrorEqual(err, goctx.DeadlineExceeded) || grpc.Code(errors.Cause(err)) == codes.DeadlineExceeded ||
if terror.ErrorEqual(err, goctx.DeadlineExceeded) || status.Code(errors.Cause(err)) == codes.DeadlineExceeded ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code returns the error code for err if it was produced by the rpc system. Otherwise, it returns codes.Unknown.
Deprecated: use status.Code instead.

https://godoc.org/google.golang.org/grpc#Code

@codecov
Copy link

codecov bot commented Nov 18, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@5d76de9). Click here to learn what that means.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #13468   +/-   ##
===========================================
  Coverage          ?   80.0788%           
===========================================
  Files             ?        473           
  Lines             ?     116620           
  Branches          ?          0           
===========================================
  Hits              ?      93388           
  Misses            ?      15888           
  Partials          ?       7344

statistics/feedback.go Outdated Show resolved Hide resolved
util/rowDecoder/decoder.go Outdated Show resolved Hide resolved
@zz-jason
Copy link
Member

BTW, this PR is conflicted with the master branch, please resolve the conflicts in your spare time.

@xiekeyi98 xiekeyi98 requested a review from a team as a code owner December 28, 2019 13:41
@ghost ghost requested review from eurekaka and francis0407 and removed request for a team December 28, 2019 13:41
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added status/LGT1 Indicates that a PR has LGTM 1. status/PTAL and removed status/ReqChange labels Jan 2, 2020
Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@winoros
Copy link
Member

winoros commented Jan 2, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 2, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jan 2, 2020

/run-all-tests

@sre-bot sre-bot merged commit 4dfbb14 into pingcap:master Jan 2, 2020
@xiekeyi98 xiekeyi98 deleted the tidyCode branch January 3, 2020 02:44
@xiekeyi98 xiekeyi98 restored the tidyCode branch April 14, 2021 12:47
@xiekeyi98 xiekeyi98 deleted the tidyCode branch April 14, 2021 12:47
@xiekeyi98 xiekeyi98 restored the tidyCode branch April 14, 2021 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor. status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants