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

[patch] remove cockroachdb/errors #677

Merged
merged 3 commits into from
Sep 8, 2020

Conversation

kpango
Copy link
Collaborator

@kpango kpango commented Sep 7, 2020

Signed-off-by: kpango i.can.feel.gravity@gmail.com

Description:

remove cockroachdb/errors for license problem

Related Issue:

How Has This Been Tested?:

Environment:

  • Go Version: 1.15.1
  • Docker Version: 19.03.8
  • Kubernetes Version: 1.18.2
  • NGT Version: 1.12.1

Types of changes:

  • Bug fix [type/bug]
  • New feature [type/feature]
  • Add tests [type/test]
  • Security related changes [type/security]
  • Add documents [type/documentation]
  • Refactoring [type/refactoring]
  • Update dependencies [type/dependency]
  • Update benchmarks and performances [type/bench]
  • Update CI [type/ci]

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Checklist:

  • I have read the CONTRIBUTING document.
  • I have checked open Pull Requests for the similar feature or fixes?
  • I have added tests and benchmarks to cover my changes.
  • I have ensured all new and existing tests passed.
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly.

Signed-off-by: kpango <i.can.feel.gravity@gmail.com>
@pull-assistant
Copy link

pull-assistant bot commented Sep 7, 2020

Score: 1.00

Best reviewed: commit by commit


Optimal code review plan (1 warning)

[patch] remove cockroachdb/errors

internal/errors/errors.go 50% changes removed in fix deepsource go

     change error message format

     fix deepsource go

Powered by Pull Assistant. Last update d23ea5a ... d79b6b4. Read the comment docs.

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Sep 7, 2020

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 💌 /changelog - add changelog comment
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • /rebase - rebase master

rinx
rinx previously approved these changes Sep 7, 2020
Copy link
Contributor

@rinx rinx left a comment

Choose a reason for hiding this comment

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

LGTM

hlts2
hlts2 previously approved these changes Sep 7, 2020
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM

}
return nil
}

Unwrap = func(err error) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
Unwrap is a global variable (gochecknoglobals)

Is = errors.Is
UnWrapOnce = errbase.UnwrapOnce
UnWrapAll = errbase.UnwrapAll
As = errors.As
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
As is a global variable (gochecknoglobals)

UnWrapOnce = errbase.UnwrapOnce
UnWrapAll = errbase.UnwrapAll
As = errors.As
Is = errors.Is
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
Is is a global variable (gochecknoglobals)

@codecov
Copy link

codecov bot commented Sep 7, 2020

Codecov Report

Merging #677 into master will increase coverage by 0.21%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #677      +/-   ##
==========================================
+ Coverage   15.38%   15.60%   +0.21%     
==========================================
  Files         412      413       +1     
  Lines       21598    21660      +62     
==========================================
+ Hits         3323     3379      +56     
- Misses      18027    18030       +3     
- Partials      248      251       +3     
Impacted Files Coverage Δ
internal/errors/blob.go 0.00% <0.00%> (ø)
internal/errors/cassandra.go 0.00% <0.00%> (ø)
internal/errors/errors.go 3.44% <0.00%> (-1.21%) ⬇️
internal/errors/mysql.go 0.00% <0.00%> (ø)
internal/errors/redis.go 0.00% <0.00%> (ø)
...g/manager/backup/cassandra/handler/grpc/handler.go 0.00% <ø> (ø)
pkg/manager/backup/mysql/handler/grpc/handler.go 0.00% <ø> (ø)
internal/net/net.go 87.30% <0.00%> (ø)
internal/worker/worker.go 85.45% <0.00%> (+0.90%) ⬆️

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 755d9c0...d79b6b4. Read the comment docs.

@kpango kpango dismissed stale reviews from hlts2 and rinx via 39b09d3 September 7, 2020 10:41
@github-actions github-actions bot added team/set SET team size/XXXL and removed size/L labels Sep 7, 2020
}
return nil
}

Unwrap = func(err error) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
unlambda: replace `func(err error) error {

@kpango kpango force-pushed the feature/internal/remove-cockroachdb-errors branch from 39b09d3 to ecfa457 Compare September 7, 2020 11:17
Signed-off-by: kpango <i.can.feel.gravity@gmail.com>
@kpango kpango force-pushed the feature/internal/remove-cockroachdb-errors branch from ecfa457 to 74920f0 Compare September 7, 2020 11:24
Is = errors.Is
UnWrapOnce = errbase.UnwrapOnce
UnWrapAll = errbase.UnwrapAll
Is = func(err, target error) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
Is is a global variable (gochecknoglobals)

@@ -101,7 +101,7 @@ func TestGet(t *testing.T) {
}
defaultCheckFunc := func(w want, got Detail) error {
if !reflect.DeepEqual(got, w.want) {
return errors.Errorf("got = %v, want %v", got, w.want)
return errors.Errorf("got: \"%#v\",\n\t\t\t\twant: \"%#v\"", got, w.want)
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
copylocks: call of errors.Errorf copies lock value: github.com/vdaas/vald/internal/info.Detail contains sync.Once contains sync.Mutex (govet)

@@ -293,7 +293,7 @@ func TestDetail_Get(t *testing.T) {
}
defaultCheckFunc := func(w want, got Detail) error {
if !reflect.DeepEqual(got, w.want) {
return errors.Errorf("got = %v, want %v", got, w.want)
return errors.Errorf("got: \"%#v\",\n\t\t\t\twant: \"%#v\"", got, w.want)
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
copylocks: call of errors.Errorf copies lock value: github.com/vdaas/vald/internal/info.Detail contains sync.Once contains sync.Mutex (govet)

}
return nil
}

Unwrap = func(err error) error { return errors.Unwrap(err) }
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
unlambda: replace func(err error) error { return errors.Unwrap(err) } with errors.Unwrap (gocritic)

@kpango kpango force-pushed the feature/internal/remove-cockroachdb-errors branch from 6a11eb5 to 7342ba4 Compare September 7, 2020 14:55
@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Sep 7, 2020

[WARNING:INTCFG] Changes in interal/config may require you to change Helm charts. Please check.

Signed-off-by: kpango <i.can.feel.gravity@gmail.com>
@kpango kpango force-pushed the feature/internal/remove-cockroachdb-errors branch from 5a42815 to d79b6b4 Compare September 7, 2020 15:19
@kpango
Copy link
Collaborator Author

kpango commented Sep 8, 2020

@rinx @hlts2 I passed all test code and deep source.
I changed errors/errors.go's Is function & test comparator error message format.
Others no changes from yesterday you reviewed, would you mind if I merge this PR?

@@ -349,8 +349,7 @@
"format": "int64"
},
"indexing": {
"type": "boolean",
"format": "boolean"
Copy link
Contributor

Choose a reason for hiding this comment

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

why these swagger files updated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

don't know, I just run make update for dependencies update.
may be swagger version ?

Copy link
Contributor

Choose a reason for hiding this comment

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

okay

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I checked swagger specification, there is not format field for boolean type.
maybe old version has format for boolean type.
FYI: https://swagger.io/specification/#data-types

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rinx (mention for FYI) and I'll merge this PR

@kpango kpango merged commit 2b3c7f7 into master Sep 8, 2020
@kpango kpango deleted the feature/internal/remove-cockroachdb-errors branch September 8, 2020 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants