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

enabled nonamedreturns linter #1033

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Conversation

hrapovd1
Copy link

@hrapovd1 hrapovd1 commented Feb 5, 2024

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: #958

What is the new behavior?

Other information

hrapovd1 and others added 4 commits February 5, 2024 22:46
* Refactored `config/defaults.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/basic/database_sql/data.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/basic/database_sql/series.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/basic/gorm/models.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/basic/native/series.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/basic/xorm/data.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/ddl/ddl.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/decimal/main.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/describe/main.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/pagination/cities.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/read_table/orders.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/serverless/healthcheck/service.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/serverless/url_shortener/service.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/topic/cdc-fill-and-read/tables.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/topic/topicreader/stubs.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/topic/topicreader/topicreader_advanced.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/topic/topicreader/topicreader_trace.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/ttl/series.go`, when golangci-lint nonamedreturns enabled
* Refactored `examples/ttl_readtable/series.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/allocator/allocator.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/backoff/backoff_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/balancer/balancer.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/balancer/connections_state.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/balancer/ctx.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/balancer/local_dc.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/bind/auto_declare.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/bind/bind.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/bind/numeric_args.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/bind/params.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/bind/positional_args.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/bind/table_path_prefix.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/certificates/certificates.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/cmd/gtrace/main.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/cmd/gtrace/writer.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/conn/conn.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/conn/grpc_client_stream.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/conn/pool.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/coordination/client.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/credentials/static.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/decimal/decimal.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/discovery/discovery.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/dsn/dsn.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/endpoint/endpoint.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/grpcwrapper/rawtopic/client.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/grpcwrapper/rawtopic/rawtopicreader/messages.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/grpcwrapper/rawtopic/rawtopicwriter/messages.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/meta/meta.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/mock/conn.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/operation/context.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/ratelimiter/client.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/repeater/repeater.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/repeater/repeater_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/scheme/client.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/scheme/helpers/check_exists.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/scheme/helpers/check_exists_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/scripting/client.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/stack/record.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/client.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/client_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/retry.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/retry_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/scanner/result.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/scanner/scan_raw.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/scanner/scanner.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/scanner/scanner_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/scanner/stats.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/session.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/session_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/statement.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/table/transaction.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/retriable_error.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicclientinternal/client.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicreaderinternal/batch.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicreaderinternal/batcher.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicreaderinternal/committer.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicreaderinternal/message.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicreaderinternal/one_time_reader.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicreaderinternal/reader.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicreaderinternal/stream_reader_impl.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicreaderinternal/stream_reconnector.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicwriterinternal/queue.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicwriterinternal/writer.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicwriterinternal/writer_reconnector.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/topic/topicwriterinternal/writer_single_stream.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/value/time.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/value/type.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/value/value.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/version/parse.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xcontext/context_with_cancel.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xcontext/context_with_timeout.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xcontext/without_deadline.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xerrors/check.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xerrors/issues.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xerrors/operation.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xresolver/xresolver.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xsql/conn.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xsql/connector.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xsql/context.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xsql/dsn.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xsql/isolation/isolation.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xsql/rows.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xsql/stmt.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xsql/tx.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xsql/tx_fake.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xsql/unwrap.go`, when golangci-lint nonamedreturns enabled
* Refactored `internal/xstring/convert.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/coordination.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/discovery.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/driver.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/field.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/ratelimiter.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/retry.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/scheme.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/scripting.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/sql.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/table.go`, when golangci-lint nonamedreturns enabled
* Refactored `log/topic.go`, when golangci-lint nonamedreturns enabled
* Refactored `meta/consumed_units.go`, when golangci-lint nonamedreturns enabled
* Refactored `meta/example_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `metrics/coordination.go`, when golangci-lint nonamedreturns enabled
* Refactored `metrics/discovery.go`, when golangci-lint nonamedreturns enabled
* Refactored `metrics/driver.go`, when golangci-lint nonamedreturns enabled
* Refactored `metrics/ratelimiter.go`, when golangci-lint nonamedreturns enabled
* Refactored `metrics/retry.go`, when golangci-lint nonamedreturns enabled
* Refactored `metrics/scheme.go`, when golangci-lint nonamedreturns enabled
* Refactored `metrics/scripting.go`, when golangci-lint nonamedreturns enabled
* Refactored `metrics/sql.go`, when golangci-lint nonamedreturns enabled
* Refactored `metrics/table.go`, when golangci-lint nonamedreturns enabled
* Refactored `retry/retry.go`, when golangci-lint nonamedreturns enabled
* Refactored `retry/retryable_error.go`, when golangci-lint nonamedreturns enabled
* Refactored `scheme/scheme.go`, when golangci-lint nonamedreturns enabled
* Refactored `scripting/example_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `sugar/certificates.go`, when golangci-lint nonamedreturns enabled
* Refactored `sugar/check_exists.go`, when golangci-lint nonamedreturns enabled
* Refactored `sugar/dsn.go`, when golangci-lint nonamedreturns enabled
* Refactored `sugar/params.go`, when golangci-lint nonamedreturns enabled
* Refactored `sugar/params_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `sugar/path.go`, when golangci-lint nonamedreturns enabled
* Refactored `table/example_test.go`, when golangci-lint nonamedreturns enabled
* Refactored `table/types/cast.go`, when golangci-lint nonamedreturns enabled
* Refactored `tests/slo/database/sql/main.go`, when golangci-lint nonamedreturns enabled
* Refactored `tests/slo/database/sql/storage.go`, when golangci-lint nonamedreturns enabled
* Refactored `tests/slo/gorm/main.go`, when golangci-lint nonamedreturns enabled
* Refactored `tests/slo/gorm/storage.go`, when golangci-lint nonamedreturns enabled
* Refactored `tests/slo/internal/workers/read.go`, when golangci-lint nonamedreturns enabled
* Refactored `tests/slo/internal/workers/write.go`, when golangci-lint nonamedreturns enabled
* Refactored `tests/slo/native/main.go`, when golangci-lint nonamedreturns enabled
* Refactored `tests/slo/native/storage.go`, when golangci-lint nonamedreturns enabled
* Refactored `tests/slo/xorm/main.go`, when golangci-lint nonamedreturns enabled
* Refactored `tests/slo/xorm/storage.go`, when golangci-lint nonamedreturns enabled
* Refactored `testutil/driver.go`, when golangci-lint nonamedreturns enabled
* Refactored `topic/topicoptions/topicoptions_alter.go`, when golangci-lint nonamedreturns enabled
* Refactored `topic/topicwriter/topicwriter.go`, when golangci-lint nonamedreturns enabled
* Refactored `trace/details.go`, when golangci-lint nonamedreturns enabled
* Refactored `trace/driver.go`, when golangci-lint nonamedreturns enabled
Copy link
Member

@rekby rekby left a comment

Choose a reason for hiding this comment

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

Hello.

Thanks for the PR.

You have a greak work for fix many typos in the code.
But now all tests are broken:
https://github.com/ydb-platform/ydb-go-sdk/actions/runs/7889717492/job/21734394246?pr=1033

usually it say about some mistake in one of common components.
Fix the bug please, then PR will be merged.

@@ -30,7 +30,8 @@ var (
}
)

func defaultGrpcOptions(t *trace.Driver, secure bool, tlsConfig *tls.Config) (opts []grpc.DialOption) {
func defaultGrpcOptions(t *trace.Driver, secure bool, tlsConfig *tls.Config) []grpc.DialOption {
var opts []grpc.DialOption
Copy link
Member

Choose a reason for hiding this comment

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

You can create a slice with predefined capacity
Because next line will re-allocate internal slice capacity

Copy link
Member

@asmyasnikov asmyasnikov Feb 20, 2024

Choose a reason for hiding this comment

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

Alternatively you can make slice from static
Such as

opts := []grpc.DialOption{
		grpc.WithKeepaliveParams(
			DefaultGrpcConnectionPolicy,
		),
		// use round robin balancing policy for fastest dialing
		grpc.WithDefaultServiceConfig(`{
			"loadBalancingPolicy": "round_robin"
		}`),
		// limit size of outgoing and incoming packages
		grpc.WithDefaultCallOptions(
			grpc.MaxCallRecvMsgSize(DefaultGRPCMsgSize),
			grpc.MaxCallSendMsgSize(DefaultGRPCMsgSize),
		),
		// use proxy-resolvers
		// 1) for interpret schemas `ydb`, `grpc` and `grpcs` in node URLs as for dns resolver
		// 2) for observe resolving events
		grpc.WithResolvers(
			xresolver.New("", t),
			xresolver.New("ydb", t),
			xresolver.New("grpc", t),
			xresolver.New("grpcs", t),
		),
}

But on next step (if secure) append will re-allocate slice to new size

Copy link
Author

Choose a reason for hiding this comment

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

I fixed this by adding the make with fixed capacity.

}

func getDataForITCrowd(seriesID string) (series types.Value, seasons, episodes []types.Value) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this changes not required and harmful
Because if I call getDataForITCrowd - I want to know about each of three results.
Old variant helps to understand. New variant cannot helps

Copy link
Author

Choose a reason for hiding this comment

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

OK. I returned previous declaration.
I simple followed by linter's comment.

@@ -120,12 +129,16 @@ func getDataForITCrowd(seriesID string) (series types.Value, seasons, episodes [
return series, seasons, episodes
}

func getDataForSiliconValley(seriesID string) (series types.Value, seasons, episodes []types.Value) {
Copy link
Member

Choose a reason for hiding this comment

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

Read previous comment

Copy link
Author

Choose a reason for hiding this comment

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

OK. I returned previous declaration.
I simple followed by linter's comment.

@@ -48,7 +48,12 @@ func episodeData(seriesID, seasonID, episodeID, title string, date time.Time) ty
)
}

func getData() (series, seasons, episodes []types.Value) {
func getData() ([]types.Value, []types.Value, []types.Value) {
Copy link
Member

Choose a reason for hiding this comment

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

Read comment for getDataForITCrowd

Copy link
Author

Choose a reason for hiding this comment

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

OK. I returned previous declaration.
I simple followed by linter's comment.

@codecov-commenter
Copy link

codecov-commenter commented Feb 27, 2024

Codecov Report

Attention: Patch coverage is 51.58730% with 732 lines in your changes are missing coverage. Please review.

Project coverage is 65.73%. Comparing base (132dfcc) to head (68e1ee6).

Files Patch % Lines
internal/value/type.go 0.00% 520 Missing ⚠️
internal/table/scanner/scan_raw.go 26.38% 53 Missing ⚠️
internal/xsql/tx_fake.go 0.00% 37 Missing ⚠️
internal/table/scanner/scanner.go 52.38% 24 Missing and 6 partials ⚠️
internal/cmd/gtrace/writer.go 0.00% 15 Missing ⚠️
internal/xsql/conn.go 84.61% 6 Missing and 6 partials ⚠️
internal/scheme/client.go 78.43% 10 Missing and 1 partial ⚠️
internal/cmd/gtrace/main.go 0.00% 7 Missing ⚠️
internal/allocator/allocator.go 94.05% 6 Missing ⚠️
internal/table/session.go 86.36% 6 Missing ⚠️
... and 18 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1033      +/-   ##
==========================================
- Coverage   66.78%   65.73%   -1.05%     
==========================================
  Files         285      286       +1     
  Lines       27908    28711     +803     
==========================================
+ Hits        18638    18873     +235     
- Misses       8407     8977     +570     
+ Partials      863      861       -2     
Flag Coverage Δ
53.28% <47.88%> (-0.83%) ⬇️
go-1.21.x 67.36% <50.30%> (-1.13%) ⬇️
go-1.22.x 65.64% <51.58%> (-1.14%) ⬇️
integration 53.28% <47.88%> (-0.83%) ⬇️
macOS 37.43% <23.28%> (-0.75%) ⬇️
ubuntu 37.46% <23.34%> (-0.70%) ⬇️
unit 37.50% <23.34%> (-0.73%) ⬇️
windows 37.46% <23.28%> (-0.73%) ⬇️
ydb-22.5 50.96% <45.93%> (-0.77%) ⬇️
ydb-23.1 50.99% <45.93%> (-0.75%) ⬇️
ydb-23.2 50.98% <45.93%> (-0.93%) ⬇️
ydb-23.3 51.11% <46.07%> (-0.59%) ⬇️
ydb-24.1 53.02% <47.88%> (-0.87%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rekby
Copy link
Member

rekby commented Feb 28, 2024

@hrapovd1 hello.
I saw you made few commits for fixes integration tests, but it still broken.

Do you need description how to run the tests at local computer - for faster check fixes and able to run a test with debugger?

@hrapovd1
Copy link
Author

Hello, @rekby!
Thank you for question. Could you please comment my way for tests which I'm using on local PC?
I'm testing in that way:

  • create dir:
    mkdir /tmp/ydb_certs
  • run ydb:
    docker run --rm -it --name ydb -p 2135:2135 -p 2136:2136 -p 8765:8765 -h localhost -e "YDB_LOCAL_SURVIVE_RESTART=true" -e "YDB_USE_IN_MEMORY_PDISKS=true" -e CI=true -v "/tmp/ydb_certs":"/ydb_certs" cr.yandex/yc/yandex-docker-local-ydb:23.2
  • In the second bash session I'm export env vars:
    GO: 1.21.x
    YDB_VERSION: 22.5
    YDB_CONNECTION_STRING: grpc://localhost:2136/local
    YDB_CONNECTION_STRING_SECURE: grpcs://localhost:2135/local
    YDB_SSL_ROOT_CERTIFICATES_FILE: /tmp/ydb_certs/ca.pem
    YDB_SESSIONS_SHUTDOWN_URLS: http://localhost:8765/actors/kqp_proxy?force_shutdown=all
    HIDE_APPLICATION_OUTPUT: 1
  • And run tests:
    cd ydb-go-sdk; go clean -testcache; go test -race -tags integration ./tests/integration"

@hrapovd1
Copy link
Author

Yesterday I've committed changes in master branch of my fork because integration tests of upstream master branch had got the same errors on my local PC. And I decided that some local problems and tests will be done in CI...

@hrapovd1
Copy link
Author

hrapovd1 commented Mar 5, 2024

@asmyasnikov , @rekby hello! Could you please check my work and merge if it's OK?

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