Skip to content
Snippets Groups Projects
Commit f763779e authored by flycash's avatar flycash
Browse files

Merge develop and resolve conflict

parents 0f64e2cd fdf51a1c
No related branches found
No related tags found
No related merge requests found
Showing
with 136 additions and 62 deletions
language: go
dist: trusty
sudo: required
# define the dependence env
language: go
os:
- linux
go:
- "1.13"
services:
- docker
env:
- GO111MODULE=on
install: true
# define ci-stage
script:
# license-check
- echo 'start license check'
- go fmt ./... && [[ -z `git status -s` ]]
- sh before_validate_license.sh
- chmod u+x /tmp/tools/license/license-header-checker
- /tmp/tools/license/license-header-checker -v -a -r -i vendor /tmp/tools/license/license.txt . go && [[ -z `git status -s` ]]
# unit-test
- echo 'start unit-test'
- chmod u+x before_ut.sh && ./before_ut.sh
- go mod vendor && go test ./... -coverprofile=coverage.txt -covermode=atomic
# integrate-test
- chmod +x integrate_test.sh && ./integrate_test.sh
after_success:
- bash <(curl -s https://codecov.io/bash)
......
......@@ -42,7 +42,7 @@ var (
availableUrl, _ = common.NewURL("dubbo://192.168.1.1:20000/com.ikurento.user.UserProvider")
)
func registerAvailable(t *testing.T, invoker *mock.MockInvoker) protocol.Invoker {
func registerAvailable(invoker *mock.MockInvoker) protocol.Invoker {
extension.SetLoadbalance("random", loadbalance.NewRandomLoadBalance)
availableCluster := NewAvailableCluster()
......@@ -60,7 +60,7 @@ func TestAvailableClusterInvokerSuccess(t *testing.T) {
defer ctrl.Finish()
invoker := mock.NewMockInvoker(ctrl)
clusterInvoker := registerAvailable(t, invoker)
clusterInvoker := registerAvailable(invoker)
mockResult := &protocol.RPCResult{Rest: rest{tried: 0, success: true}}
invoker.EXPECT().IsAvailable().Return(true)
......@@ -76,7 +76,7 @@ func TestAvailableClusterInvokerNoAvail(t *testing.T) {
defer ctrl.Finish()
invoker := mock.NewMockInvoker(ctrl)
clusterInvoker := registerAvailable(t, invoker)
clusterInvoker := registerAvailable(invoker)
invoker.EXPECT().IsAvailable().Return(false)
......
......@@ -87,7 +87,6 @@ func (invoker *baseClusterInvoker) checkWhetherDestroyed() error {
}
func (invoker *baseClusterInvoker) doSelect(lb cluster.LoadBalance, invocation protocol.Invocation, invokers []protocol.Invoker, invoked []protocol.Invoker) protocol.Invoker {
var selectedInvoker protocol.Invoker
url := invokers[0].GetUrl()
sticky := url.GetParamBool(constant.STICKY_KEY, false)
......
......@@ -107,8 +107,8 @@ func normalInvoke(t *testing.T, successCount int, urlParam url.Values, invocatio
invokers := []protocol.Invoker{}
for i := 0; i < 10; i++ {
url, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.%v:20000/com.ikurento.user.UserProvider", i), common.WithParams(urlParam))
invokers = append(invokers, NewMockInvoker(url, successCount))
newUrl, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.%v:20000/com.ikurento.user.UserProvider", i), common.WithParams(urlParam))
invokers = append(invokers, NewMockInvoker(newUrl, successCount))
}
staticDir := directory.NewStaticDirectory(invokers)
......
......@@ -92,7 +92,7 @@ func (dir *BaseDirectory) SetRouters(urls []*common.URL) {
factory := extension.GetRouterFactory(url.Protocol)
r, err := factory.NewRouter(url)
if err != nil {
logger.Errorf("Create router fail. router key: %s, error: %v", routerKey, url.Service(), err)
logger.Errorf("Create router fail. router key: %s, url:%s, error: %+v", routerKey, url.Service(), err)
return
}
routers = append(routers, r)
......
......@@ -19,7 +19,6 @@ package directory
import (
"encoding/base64"
"fmt"
"testing"
)
......@@ -35,7 +34,7 @@ import (
)
func TestNewBaseDirectory(t *testing.T) {
url, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.1:20000/com.ikurento.user.UserProvider"))
url, _ := common.NewURL("dubbo://192.168.1.1:20000/com.ikurento.user.UserProvider")
directory := NewBaseDirectory(&url)
assert.NotNil(t, directory)
......@@ -46,7 +45,7 @@ func TestNewBaseDirectory(t *testing.T) {
}
func TestBuildRouterChain(t *testing.T) {
url, _ := common.NewURL(fmt.Sprintf("dubbo://192.168.1.1:20000/com.ikurento.user.UserProvider"))
url, _ := common.NewURL("dubbo://192.168.1.1:20000/com.ikurento.user.UserProvider")
directory := NewBaseDirectory(&url)
assert.NotNil(t, directory)
......
......@@ -61,7 +61,7 @@ func (dir *staticDirectory) IsAvailable() bool {
// List List invokers
func (dir *staticDirectory) List(invocation protocol.Invocation) []protocol.Invoker {
l := len(dir.invokers)
invokers := make([]protocol.Invoker, l, l)
invokers := make([]protocol.Invoker, l)
copy(invokers, dir.invokers)
routerChain := dir.RouterChain()
......
......@@ -18,7 +18,6 @@
package common
import (
"bytes"
"encoding/base64"
"fmt"
"math"
......@@ -293,18 +292,18 @@ func isMatchCategory(category1 string, category2 string) bool {
}
func (c URL) String() string {
var buildString string
var buf strings.Builder
if len(c.Username) == 0 && len(c.Password) == 0 {
buildString = fmt.Sprintf(
buf.WriteString(fmt.Sprintf(
"%s://%s:%s%s?",
c.Protocol, c.Ip, c.Port, c.Path)
c.Protocol, c.Ip, c.Port, c.Path))
} else {
buildString = fmt.Sprintf(
buf.WriteString(fmt.Sprintf(
"%s://%s:%s@%s:%s%s?",
c.Protocol, c.Username, c.Password, c.Ip, c.Port, c.Path)
c.Protocol, c.Username, c.Password, c.Ip, c.Port, c.Path))
}
buildString += c.params.Encode()
return buildString
buf.WriteString(c.params.Encode())
return buf.String()
}
// Key ...
......@@ -321,7 +320,7 @@ func (c URL) ServiceKey() string {
if intf == "" {
return ""
}
buf := &bytes.Buffer{}
var buf strings.Builder
group := c.GetParam(constant.GROUP_KEY, "")
if group != "" {
buf.WriteString(group)
......@@ -346,7 +345,7 @@ func (c *URL) ColonSeparatedKey() string {
if intf == "" {
return ""
}
buf := &bytes.Buffer{}
var buf strings.Builder
buf.WriteString(intf)
buf.WriteString(":")
version := c.GetParam(constant.VERSION_KEY, "")
......@@ -408,8 +407,6 @@ func (c *URL) RangeParams(f func(key, value string) bool) {
// GetParam ...
func (c URL) GetParam(s string, d string) string {
// c.paramsLock.RLock()
// defer c.paramsLock.RUnlock()
r := c.params.Get(s)
if len(r) == 0 {
r = d
......@@ -424,8 +421,6 @@ func (c URL) GetParams() url.Values {
// GetParamAndDecoded ...
func (c URL) GetParamAndDecoded(key string) (string, error) {
// c.paramsLock.RLock()
// defer c.paramsLock.RUnlock()
ruleDec, err := base64.URLEncoding.DecodeString(c.GetParam(key, ""))
value := string(ruleDec)
return value, err
......
......@@ -21,9 +21,11 @@ import (
"reflect"
"testing"
)
import (
"github.com/stretchr/testify/assert"
)
import (
"github.com/apache/dubbo-go/common/config"
"github.com/apache/dubbo-go/common/extension"
......@@ -481,7 +483,6 @@ func Test_refreshProvider(t *testing.T) {
}
func Test_startConfigCenter(t *testing.T) {
extension.SetConfigCenterFactory("mock", func() config_center.DynamicConfigurationFactory {
return &config_center.MockDynamicConfigurationFactory{}
})
......@@ -499,21 +500,21 @@ func Test_startConfigCenter(t *testing.T) {
}
func Test_initializeStruct(t *testing.T) {
consumerConfig := &ConsumerConfig{}
testConsumerConfig := &ConsumerConfig{}
tp := reflect.TypeOf(ConsumerConfig{})
v := reflect.New(tp)
initializeStruct(tp, v.Elem())
fmt.Println(reflect.ValueOf(consumerConfig).Elem().Type().String())
fmt.Println(reflect.ValueOf(testConsumerConfig).Elem().Type().String())
fmt.Println(v.Elem().Type().String())
reflect.ValueOf(consumerConfig).Elem().Set(v.Elem())
reflect.ValueOf(testConsumerConfig).Elem().Set(v.Elem())
assert.Condition(t, func() (success bool) {
return consumerConfig.ApplicationConfig != nil
return testConsumerConfig.ApplicationConfig != nil
})
assert.Condition(t, func() (success bool) {
return consumerConfig.Registries != nil
return testConsumerConfig.Registries != nil
})
assert.Condition(t, func() (success bool) {
return consumerConfig.References != nil
return testConsumerConfig.References != nil
})
}
......@@ -129,7 +129,7 @@ func configCenterRefreshConsumer() error {
var err error
if consumerConfig.ConfigCenterConfig != nil {
consumerConfig.SetFatherConfig(consumerConfig)
if err := consumerConfig.startConfigCenter(); err != nil {
if err = consumerConfig.startConfigCenter(); err != nil {
return perrors.Errorf("start config center error , error message is {%v}", perrors.WithStack(err))
}
consumerConfig.fresh()
......@@ -144,6 +144,5 @@ func configCenterRefreshConsumer() error {
return perrors.WithMessagef(err, "time.ParseDuration(Connect_Timeout{%#v})", consumerConfig.Connect_Timeout)
}
}
return nil
}
......@@ -154,9 +154,9 @@ func (c *ServiceConfig) Export() error {
// registry the service reflect
methods, err := common.ServiceMap.Register(c.InterfaceName, proto.Name, c.rpcService)
if err != nil {
err := perrors.Errorf("The service %v export the protocol %v error! Error message is %v .", c.InterfaceName, proto.Name, err.Error())
logger.Errorf(err.Error())
return err
formatErr := perrors.Errorf("The service %v export the protocol %v error! Error message is %v .", c.InterfaceName, proto.Name, err.Error())
logger.Errorf(formatErr.Error())
return formatErr
}
port := proto.Port
......
......@@ -124,7 +124,7 @@ func (hf *HystrixFilter) Invoke(ctx context.Context, invoker protocol.Invoker, i
_, _, err := hystrix.GetCircuit(cmdName)
configLoadMutex.RUnlock()
if err != nil {
logger.Errorf("[Hystrix Filter]Errors occurred getting circuit for %s , will invoke without hystrix, error is: ", cmdName, err)
logger.Errorf("[Hystrix Filter]Errors occurred getting circuit for %s , will invoke without hystrix, error is: %+v", cmdName, err)
return invoker.Invoke(ctx, invocation)
}
logger.Infof("[Hystrix Filter]Using hystrix filter: %s", cmdName)
......
......@@ -53,10 +53,10 @@ func TestTokenFilter_Invoke(t *testing.T) {
func TestTokenFilter_InvokeEmptyToken(t *testing.T) {
filter := GetTokenFilter()
url := common.URL{}
testUrl := common.URL{}
attch := make(map[string]string, 0)
attch[constant.TOKEN_KEY] = "ori_key"
result := filter.Invoke(context.Background(), protocol.NewBaseInvoker(url), invocation.NewRPCInvocation("MethodName", []interface{}{"OK"}, attch))
result := filter.Invoke(context.Background(), protocol.NewBaseInvoker(testUrl), invocation.NewRPCInvocation("MethodName", []interface{}{"OK"}, attch))
assert.Nil(t, result.Error())
assert.Nil(t, result.Result())
}
......@@ -64,23 +64,23 @@ func TestTokenFilter_InvokeEmptyToken(t *testing.T) {
func TestTokenFilter_InvokeEmptyAttach(t *testing.T) {
filter := GetTokenFilter()
url := common.NewURLWithOptions(
testUrl := common.NewURLWithOptions(
common.WithParams(url.Values{}),
common.WithParamsValue(constant.TOKEN_KEY, "ori_key"))
attch := make(map[string]string, 0)
result := filter.Invoke(context.Background(), protocol.NewBaseInvoker(*url), invocation.NewRPCInvocation("MethodName", []interface{}{"OK"}, attch))
result := filter.Invoke(context.Background(), protocol.NewBaseInvoker(*testUrl), invocation.NewRPCInvocation("MethodName", []interface{}{"OK"}, attch))
assert.NotNil(t, result.Error())
}
func TestTokenFilter_InvokeNotEqual(t *testing.T) {
filter := GetTokenFilter()
url := common.NewURLWithOptions(
testUrl := common.NewURLWithOptions(
common.WithParams(url.Values{}),
common.WithParamsValue(constant.TOKEN_KEY, "ori_key"))
attch := make(map[string]string, 0)
attch[constant.TOKEN_KEY] = "err_key"
result := filter.Invoke(context.Background(),
protocol.NewBaseInvoker(*url), invocation.NewRPCInvocation("MethodName", []interface{}{"OK"}, attch))
protocol.NewBaseInvoker(*testUrl), invocation.NewRPCInvocation("MethodName", []interface{}{"OK"}, attch))
assert.NotNil(t, result.Error())
}
......@@ -53,6 +53,7 @@ require (
github.com/toolkits/concurrent v0.0.0-20150624120057-a4371d70e3e3 // indirect
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 // indirect
github.com/zouyx/agollo v0.0.0-20191114083447-dde9fc9f35b8
go.etcd.io/bbolt v1.3.4 // indirect
go.uber.org/atomic v1.4.0
go.uber.org/zap v1.10.0
google.golang.org/grpc v1.22.1
......
......@@ -502,6 +502,8 @@ github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 h1:eY9dn8+vbi4tKz5
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
github.com/zouyx/agollo v0.0.0-20191114083447-dde9fc9f35b8 h1:k8TV7Gz7cpWpOw/dz71fx8cCZdWoPuckHJ/wkJl+meg=
github.com/zouyx/agollo v0.0.0-20191114083447-dde9fc9f35b8/go.mod h1:S1cAa98KMFv4Sa8SbJ6ZtvOmf0VlgH0QJ1gXI0lBfBY=
go.etcd.io/bbolt v1.3.4 h1:hi1bXHMVrlQh6WwxAy+qZCV/SYIlqo+Ushwdpa4tAKg=
go.etcd.io/bbolt v1.3.4/go.mod h1:G5EMThwa9y8QZGBClrRx5EY+Yw9kAhnjy3bSjsnlVTQ=
go.uber.org/atomic v1.4.0 h1:cxzIVoETapQEqDhQu3QfnvXAV4AlzcvUCxkVUFw3+EU=
go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
go.uber.org/multierr v1.1.0 h1:HoEmRHQPVSqub6w2z2d2EOVs2fjyFRGyofhKuyDq0QI=
......@@ -548,6 +550,8 @@ golang.org/x/sys v0.0.0-20190508220229-2d0786266e9c/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20190523142557-0e01d883c5c5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190801041406-cbf593c0f2f3 h1:4y9KwBHBgBNwDbtu44R5o1fdOCQUEXhbk/P4A9WmJq0=
golang.org/x/sys v0.0.0-20190801041406-cbf593c0f2f3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5 h1:LfCXLvNmTYH9kEmVgqbnsWfruoXZIrh4YBgqVHtDvw0=
golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.0.0-20160726164857-2910a502d2bf/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs=
......
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#!/bin/bash
set -e
set -x
echo 'start integrate-test'
# set root workspace
ROOT_DIR=$(pwd)
echo "integrate-test root work-space -> ${ROOT_DIR}"
# show all travis-env
echo "travis current commit id -> ${TRAVIS_COMMIT}"
echo "travis pull request -> ${TRAVIS_PULL_REQUEST}"
echo "travis pull request branch -> ${TRAVIS_PULL_REQUEST_BRANCH}"
echo "travis pull request slug -> ${TRAVIS_PULL_REQUEST_SLUG}"
echo "travis pull request sha -> ${TRAVIS_PULL_REQUEST_SHA}"
echo "travis pull request repo slug -> ${TRAVIS_REPO_SLUG}"
# #start etcd registry insecure listen in [:]:2379
# docker run -d --network host k8s.gcr.io/etcd:3.3.10 etcd
# echo "etcdv3 listen in [:]2379"
# #start consul registry insecure listen in [:]:8500
# docker run -d --network host consul
# echo "consul listen in [:]8500"
# #start nacos registry insecure listen in [:]:8848
# docker run -d --network host nacos/nacos-server:latest
# echo "ncacos listen in [:]8848"
# default use zk as registry
#start zookeeper registry insecure listen in [:]:2181
docker run -d --network host zookeeper
echo "zookeeper listen in [:]2181"
# build go-server image
cd ./test/integrate/dubbo/go-server
docker build . -t ci-provider --build-arg PR_ORIGIN_REPO=${TRAVIS_PULL_REQUEST_SLUG} --build-arg PR_ORIGIN_COMMITID=${TRAVIS_PULL_REQUEST_SHA}
cd ${ROOT_DIR}
docker run -d --network host ci-provider
# build go-client image
cd ./test/integrate/dubbo/go-client
docker build . -t ci-consumer --build-arg PR_ORIGIN_REPO=${TRAVIS_PULL_REQUEST_SLUG} --build-arg PR_ORIGIN_COMMITID=${TRAVIS_PULL_REQUEST_SHA}
cd ${ROOT_DIR}
# run provider
# check consumer status
docker run -it --network host ci-consumer
......@@ -143,7 +143,7 @@ func (h *RpcClientHandler) OnMessage(session getty.Session, pkg interface{}) {
// OnCron ...
func (h *RpcClientHandler) OnCron(session getty.Session) {
rpcSession, err := h.conn.getClientRpcSession(session)
clientRpcSession, err := h.conn.getClientRpcSession(session)
if err != nil {
logger.Errorf("client.getClientSession(session{%s}) = error{%v}",
session.Stat(), perrors.WithStack(err))
......@@ -151,7 +151,7 @@ func (h *RpcClientHandler) OnCron(session getty.Session) {
}
if h.conn.pool.rpcClient.conf.sessionTimeout.Nanoseconds() < time.Since(session.GetActive()).Nanoseconds() {
logger.Warnf("session{%s} timeout{%s}, reqNum{%d}",
session.Stat(), time.Since(session.GetActive()).String(), rpcSession.reqNum)
session.Stat(), time.Since(session.GetActive()).String(), clientRpcSession.reqNum)
h.conn.removeSession(session) // -> h.conn.close() -> h.conn.pool.remove(h.conn)
return
}
......
......@@ -219,25 +219,25 @@ func (c *gettyRPCClient) updateSession(session getty.Session) {
func (c *gettyRPCClient) getClientRpcSession(session getty.Session) (rpcSession, error) {
var (
err error
rpcSession rpcSession
err error
rpcClientSession rpcSession
)
c.lock.RLock()
defer c.lock.RUnlock()
if c.sessions == nil {
return rpcSession, errClientClosed
return rpcClientSession, errClientClosed
}
err = errSessionNotExist
for _, s := range c.sessions {
if s.session == session {
rpcSession = *s
rpcClientSession = *s
err = nil
break
}
}
return rpcSession, perrors.WithStack(err)
return rpcClientSession, perrors.WithStack(err)
}
func (c *gettyRPCClient) isAvailable() bool {
......@@ -319,7 +319,8 @@ func (p *gettyRPCClientPool) getGettyRpcClient(protocol, addr string) (*gettyRPC
conn, err := p.get()
if err == nil && conn == nil {
// create new conn
rpcClientConn, err := newGettyRPCClientConn(p, protocol, addr)
var rpcClientConn *gettyRPCClient
rpcClientConn, err = newGettyRPCClientConn(p, protocol, addr)
return rpcClientConn, perrors.WithStack(err)
}
return conn, perrors.WithStack(err)
......
......@@ -172,7 +172,7 @@ func (c *HTTPClient) Do(addr, path string, httpHeader http.Header, body []byte)
httpReq.Close = true
reqBuf := bytes.NewBuffer(make([]byte, 0))
if err := httpReq.Write(reqBuf); err != nil {
if err = httpReq.Write(reqBuf); err != nil {
return nil, perrors.WithStack(err)
}
......@@ -191,7 +191,7 @@ func (c *HTTPClient) Do(addr, path string, httpHeader http.Header, body []byte)
}
setNetConnTimeout(tcpConn, c.options.HTTPTimeout)
if _, err := reqBuf.WriteTo(tcpConn); err != nil {
if _, err = reqBuf.WriteTo(tcpConn); err != nil {
return nil, perrors.WithStack(err)
}
......
......@@ -67,8 +67,8 @@ type Error struct {
func (e *Error) Error() string {
buf, err := json.Marshal(e)
if err != nil {
msg, err := json.Marshal(err.Error())
if err != nil {
msg, retryErr := json.Marshal(err.Error())
if retryErr != nil {
msg = []byte("jsonrpc2.Error: json.Marshal failed")
}
return fmt.Sprintf(`{"code":%d,"message":%s}`, -32001, string(msg))
......@@ -133,7 +133,7 @@ func (c *jsonClientCodec) Write(d *CodecData) ([]byte, error) {
}
case reflect.Array, reflect.Struct:
case reflect.Ptr:
switch k := reflect.TypeOf(param).Elem().Kind(); k {
switch ptrK := reflect.TypeOf(param).Elem().Kind(); ptrK {
case reflect.Map:
if reflect.TypeOf(param).Elem().Key().Kind() == reflect.String {
if reflect.ValueOf(param).Elem().IsNil() {
......@@ -146,7 +146,7 @@ func (c *jsonClientCodec) Write(d *CodecData) ([]byte, error) {
}
case reflect.Array, reflect.Struct:
default:
return nil, perrors.New("unsupported param type: Ptr to " + k.String())
return nil, perrors.New("unsupported param type: Ptr to " + ptrK.String())
}
default:
return nil, perrors.New("unsupported param type: " + k.String())
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment