You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2020/07/07 02:53:42 UTC

[GitHub] [servicecomb-service-center] aseTo2016 commented on a change in pull request #660: add more action for code quality (#657)

aseTo2016 commented on a change in pull request #660:
URL: https://github.com/apache/servicecomb-service-center/pull/660#discussion_r450574328



##########
File path: pkg/rest/client.go
##########
@@ -129,11 +129,15 @@ func (client *URLClient) HttpDoWithContext(ctx context.Context, method string, r
 
 	DumpResponse(resp)
 
-	switch resp.Header.Get(HEADER_CONTENT_ENCODING) {
+	switch resp.Header.Get(HeaderContentEncoding) {
 	case "gzip":
 		reader, err := NewGZipBodyReader(resp.Body)
 		if err != nil {
-			io.Copy(ioutil.Discard, resp.Body)
+			_, err = io.Copy(ioutil.Discard, resp.Body)
+			if err != nil {
+				log.Error("", err)
+				return nil, err
+			}
 			resp.Body.Close()

Review comment:
       return 后,body没有close

##########
File path: server/plugin/discovery/servicecenter/syncer.go
##########
@@ -55,45 +55,50 @@ func (c *Syncer) Sync(ctx context.Context) {
 	if len(errs) > 0 {
 		err := fmt.Errorf("%v", errs)
 		log.Errorf(err, "Sync catches errors")
-		alarm.Raise(alarm.IdBackendConnectionRefuse,
+		err = alarm.Raise(alarm.IDBackendConnectionRefuse,
 			alarm.AdditionalContext(err.Error()))
+		if err != nil {
+			log.Error("", err)
+		}
 		if cache == nil {
 			return
 		}
 	}
-	alarm.Clear(alarm.IdBackendConnectionRefuse)
-
+	err := alarm.Clear(alarm.IDBackendConnectionRefuse)

Review comment:
       建议封装个ClearIgnoreError函数,返回出来,只是打印日志,不如封装个函数

##########
File path: scctl/pkg/version/version.go
##########
@@ -30,14 +30,14 @@ var (
 	TOOL_NAME = "scctl"

Review comment:
       TOOL_NAME 这个是不是也可以修改下?

##########
File path: server/service/kv/store.go
##########
@@ -70,3 +70,14 @@ func Exist(ctx context.Context, key string) (bool, error) {
 	}
 	return true, nil
 }
+func Delete(ctx context.Context, key string) (bool, error) {
+	resp, err := backend.Registry().Do(ctx, registry.DEL,
+		registry.WithStrKey(key))
+	if err != nil {
+		return false, err
+	}
+	if resp.Count == 0 {

Review comment:
       直接return resp.Count != 0




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org