Copilot commented on code in PR #922:
URL: https://github.com/apache/dubbo-go-samples/pull/922#discussion_r2443329627
##########
.github/workflows/github-actions.yml:
##########
@@ -4,21 +4,34 @@ on:
push:
branches:
- main
- - 'release-*'
- - 'feature-*'
+ - "release-*"
+ - "feature-*"
pull_request:
- branches: "*"
+ branches: ["*"]
jobs:
+ license:
+ name: Check License Header
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
- build:
+ - name: Check License Header
+ uses: apache/skywalking-eyes/header@main #NOSONAR
+ env:
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ with:
+ config: .licenserc.yaml
Review Comment:
Pin the action to a specific version tag or commit SHA instead of 'main' to
avoid supply-chain risks from upstream changes, e.g.,
'apache/skywalking-eyes/header@<tag or sha>'.
##########
.golangci.yml:
##########
@@ -1,46 +1,85 @@
-linters-settings:
- gocyclo:
- min-complexity: 10
- dupl:
- threshold: 100
- goconst:
- min-len: 2
- min-occurrences: 2
- depguard:
- rules:
- main:
- deny:
- - pkg: "github.com/sirupsen/logrus"
- desc: logging is allowed only by logutils.Log, logrus, is allowed
to use only in logutils package
- lll:
- line-length: 140
- goimports:
- local-prefixes: github.com/golangci/golangci-lint
- gocritic:
- enabled-tags:
- - performance
- - style
- - experimental
- disabled-checks:
- - wrapperFunc
+# 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.
+version: "2"
linters:
- disable-all: true
+ default: none
enable:
- - staticcheck
+ - govet
- ineffassign
- misspell
-
-issues:
- exclude-dirs:
- - test/testdata_etc
- - pkg/golinters/goanalysis/(checker|passes)
- exclude-rules:
- - text: "weak cryptographic primitive"
- linters:
- - gosec
- - linters:
- - staticcheck
- text: "SA1019:"
-
-
+ - staticcheck
+ - unused
+ settings:
+ depguard:
Review Comment:
In golangci-lint v2, linter settings must be defined under the top-level
'settings' key, not nested under 'linters'. Move the entire 'settings' block
(depguard, dupl, goconst, gocritic, gocyclo, govet, lll, misspell, etc.) out to
a top-level 'settings' section so they are applied.
##########
context/go-server/cmd/main.go:
##########
@@ -40,7 +40,7 @@ type GreetTripleServer struct {
func (srv *GreetTripleServer) Greet(ctx context.Context, req
*greet.GreetRequest) (*greet.GreetResponse, error) {
// map must be assert to map[string]interface, because of dubbo
limitation
Review Comment:
The comment references map[string]interface but the code asserts to
map[string]any. Update the comment to match (or note that any is an alias of
interface{}) to avoid confusion.
```suggestion
// map must be asserted to map[string]any (any is an alias for
interface{}), because of dubbo limitation
```
##########
.github/workflows/github-actions.yml:
##########
@@ -27,42 +40,36 @@ jobs:
DING_SIGN: ${{ secrets.DING_SIGN }}
steps:
- - name: Set up Go 1.x
- uses: actions/setup-go@v2
+ - name: Setup Go ${{ matrix.go_version }}
+ uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go_version }}
- id: go
- - name: Check out code into the Go module directory
+ - name: Checkout
uses: actions/[email protected]
- name: Cache dependencies
+ # ref:
https://github.com/actions/cache/blob/main/examples.md#go---module
uses: actions/cache@v4
with:
- # Cache
- path: ~/go/pkg/mod
+ # Cache, works only on Linux
+ path: |
+ ~/.cache/go-build
+ ~/go/pkg/mod
# Cache key
- key: ${{ runner.os }}-go-${{ matrix.go_version }}-${{
hashFiles('**/go.sum') }}
+ key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
# An ordered list of keys to use for restoring the cache if no cache
hit occurred for key
restore-keys: |
- ${{ runner.os }}-go-${{ matrix.go_version }}-
+ ${{ runner.os }}-go-
Review Comment:
Include the Go version in the cache key to avoid cross-version cache
collisions if/when multiple Go versions are added to the matrix (e.g., 'key:
${{ runner.os }}-go-${{ matrix.go_version }}-${{ hashFiles('**/go.sum') }}').
##########
compatibility/context/triple/go-server/cmd/server.go:
##########
@@ -62,7 +62,7 @@ func (s *GreeterProvider) SayHello(ctx context.Context, in
*api.HelloRequest) (*
func (s *GreeterProvider) SayHelloStream(svr api.Greeter_SayHelloStreamServer)
error {
// map must be assert to map[string]interface, because of dubbo
limitation
Review Comment:
Same as above: update the comment to reference map[string]any (or clarify
the alias) to keep docs consistent with code.
```suggestion
// map must be asserted to map[string]any, because of dubbo limitation
```
##########
compatibility/context/triple/go-server/cmd/server.go:
##########
@@ -41,7 +41,7 @@ type GreeterProvider struct {
func (s *GreeterProvider) SayHello(ctx context.Context, in *api.HelloRequest)
(*api.User, error) {
// map must be assert to map[string]interface, because of dubbo
limitation
- attachments :=
ctx.Value(constant.AttachmentKey).(map[string]interface{})
+ attachments := ctx.Value(constant.AttachmentKey).(map[string]any)
Review Comment:
Comment mentions map[string]interface while the code uses map[string]any.
Please align the comment with the code (any is an alias of interface{}).
##########
.github/workflows/github-actions.yml:
##########
@@ -27,42 +40,36 @@ jobs:
DING_SIGN: ${{ secrets.DING_SIGN }}
steps:
- - name: Set up Go 1.x
- uses: actions/setup-go@v2
+ - name: Setup Go ${{ matrix.go_version }}
+ uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go_version }}
- id: go
- - name: Check out code into the Go module directory
+ - name: Checkout
uses: actions/[email protected]
- name: Cache dependencies
+ # ref:
https://github.com/actions/cache/blob/main/examples.md#go---module
uses: actions/cache@v4
with:
- # Cache
- path: ~/go/pkg/mod
+ # Cache, works only on Linux
+ path: |
+ ~/.cache/go-build
+ ~/go/pkg/mod
# Cache key
- key: ${{ runner.os }}-go-${{ matrix.go_version }}-${{
hashFiles('**/go.sum') }}
+ key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
# An ordered list of keys to use for restoring the cache if no cache
hit occurred for key
restore-keys: |
- ${{ runner.os }}-go-${{ matrix.go_version }}-
+ ${{ runner.os }}-go-
- - name: Get dependencies
- run: |
- go mod tidy
- # for imports-formatter
- go install github.com/dubbogo/tools/cmd/imports-formatter@latest
-
- - name: Format Code
- run: |
- go fmt ./... && GOROOT=$(go env GOROOT) imports-formatter && git
status && [[ -z `git status -s` ]]
- # diff -u <(echo -n) <(gofmt -d -s .)
+ - name: Check Code Format
+ run: make fmt && git status && [[ -z `git status -s` ]]
- - name: Check License Header
- uses:
apache/skywalking-eyes/header@e1a02359b239bd28de3f6d35fdc870250fa513d5
+ - name: Lint
+ run: make lint
- name: Set Go version again to override skywalking
- uses: actions/setup-go@v2
+ uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go_version }}
Review Comment:
This step's description references 'override skywalking', but the license
check now runs in a separate job; the step is redundant. Remove this step or
update its name to reflect its purpose.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
