-
Notifications
You must be signed in to change notification settings - Fork 281
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
Change - Documentation Update #1380
base: development
Are you sure you want to change the base?
Conversation
@nidhi752 Still there are some linter errors in your PR. kindly have a look at the workflow run : https://github.com/gofr-dev/gofr/actions/runs/12684562987?pr=1380 and resolve the linters |
func New(config Config) *Client { | ||
return &Client{config: config} | ||
} | ||
|
||
// UseLogger sets the logger for the Clickhouse client. | ||
// The logger must implement the Logger interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use godoc link
// The logger must implement the Logger interface. | |
// The logger must implement the [Logger] interface. |
Also the must is a bit strong as you can pass something that doesn't, it would be ignored
func (c *Client) UseLogger(logger interface{}) { | ||
if l, ok := logger.(Logger); ok { | ||
c.logger = l | ||
} | ||
} | ||
|
||
// UseMetrics sets the metrics for the Clickhouse client. | ||
// The metrics must implement the Metrics interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// The metrics must implement the Metrics interface. | |
// The metrics must implement the [Metrics] interface. |
And same remark about "must"
) | ||
|
||
|
||
// Config defines the configuration needed to connect to a Clickhouse database. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Config defines the configuration needed to connect to a Clickhouse database. | |
// Config defines the configuration needed to connect to a ClickHouse database. |
} | ||
|
||
// Client represents the Clickhouse client, encapsulating connection details, configuration, logging, metrics, and tracing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Client represents the Clickhouse client, encapsulating connection details, configuration, logging, metrics, and tracing. | |
// Client represents the ClickHouse client, encapsulating connection details, configuration, logging, metrics, and tracing. |
// client.UseMetrics(Metrics()) | ||
// | ||
// client.Connect() | ||
// New initializes a new Clickhouse client with the provided configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// New initializes a new Clickhouse client with the provided configuration. | |
// New initializes a new ClickHouse client with the provided configuration. |
func (c *Client) UseMetrics(metrics interface{}) { | ||
if m, ok := metrics.(Metrics); ok { | ||
c.metrics = m | ||
} | ||
} | ||
|
||
// UseTracer sets the tracer for Clickhouse client. | ||
// UseTracer sets the OpenTelemetry tracer for the Clickhouse client. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// UseTracer sets the OpenTelemetry tracer for the Clickhouse client. | |
// UseTracer sets the OpenTelemetry tracer for the ClickHouse client. |
func (c *Client) UseTracer(tracer any) { | ||
if t, ok := tracer.(trace.Tracer); ok { | ||
c.tracer = t | ||
} | ||
} | ||
|
||
// Connect establishes a connection to Clickhouse and registers metrics using the provided configuration when the client was Created. | ||
// Connect establishes a connection to the Clickhouse database using the client's configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Connect establishes a connection to the Clickhouse database using the client's configuration. | |
// Connect establishes a connection to the ClickHouse database using the client's configuration. |
@nidhi752 Please address the review comments given in the PR. |
@Umang01-hash |
Yes @nidhi752 @ccoVeille has requested a few please check |
@nidhi752 Requesting you to please work on the review comments else we will need to close the PR due to inactivity. |
Pull Request Template
Description:
Checklist:
goimport
andgolangci-lint
.Thank you for your contribution!