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

Fix Code format (including comment) #565

Merged
merged 1 commit into from Feb 15, 2023
Merged

Conversation

fengshunli
Copy link
Contributor

Signed-off-by: fengshunli 1171313930@qq.com

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduced a user-facing change?


Additional documentation, usage docs, etc.:


wenchajun
wenchajun previously approved these changes Feb 13, 2023
benjaminhuo
benjaminhuo previously approved these changes Feb 15, 2023
@@ -30,7 +30,7 @@ type LabelRouter struct {
Routes []*Route `json:"routes,omitempty"`
}

// Each fluentd config instance will create a route pluginstore.
// NewRoutePlugin Each fluentd config instance will create a route pluginstore.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// NewRoutePlugin Each fluentd config instance will create a route pluginstore.
// Each fluentd config instance will create a route pluginstore.

Copy link
Member

Choose a reason for hiding this comment

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

No need to add function names like NewRoutePlugin and NewGlobalRouter to the function's comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the function or parameter name as the beginning of the comment is correct, but we need to ensure that the comment is grammatical. .

Copy link
Member

Choose a reason for hiding this comment

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

Seems that both comments are not grammatical:

NewRoutePlugin Each fluentd config instance will create a route pluginstore.
NewGlobalRouter The global router to store routes Member

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Member

Choose a reason for hiding this comment

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

NewRoutePlugin Each fluentd config instance will create a route pluginstore.
NewGlobalRouter The global router to store routes Member

I mean we cannot simply append a function name to the original comments to conform with the guideline, we need to make the entire comment conform the grammar maybe like this:

NewRoutePlugin creates a route pluginstore for each fluentd config instance.
NewGlobalRouter creates a global router to store routes Member

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@@ -83,7 +83,7 @@ func (r *Route) NewRoutePlugin() (*params.PluginStore, error) {
return ps, nil
}

// The global router to store routes
// NewGlobalRouter The global router to store routes
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// NewGlobalRouter The global router to store routes
// The global router to store routes

pkg/operator/collector-statefulset.go Outdated Show resolved Hide resolved
Signed-off-by: fengshunli <1171313930@qq.com>
@benjaminhuo
Copy link
Member

Thanks for the contribution!

@benjaminhuo benjaminhuo merged commit 477be7d into fluent:master Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants