*: sort-statement-order

[milasuperstar]

Signed-off-by: Mila Wu wumila1996@gmail.com

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:
In the uber golang style, there are two rules:

• Functions should be sorted in rough call order.

• Functions in a file should be grouped by receiver.

I think we should follow this rule.

What is changed and how it works?

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No code

Release note

None

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has been approved by:

• JmPotato
• disksing

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

[CLAassistant]

All committers have signed the CLA.

[ti-chi-bot]

@hawkingrei: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

[hawkingrei]

LGTM

[milasuperstar]

In the uber golang style, there are two rules:

• Functions should be sorted in rough call order.

• Functions in a file should be grouped by receiver.

I think we should follow this rule.

[JmPotato]

BTW, could you please finish the PR description to explain these changes a little bit? That would be nice for other reviewers to review. :)

[disksing]

/merge

[ti-chi-bot]

This pull request has been accepted and is ready to merge.

Commit hash: 9376f1d

[milasuperstar]

BTW, could you please finish the PR description to explain these changes a little bit? That would be nice for other reviewers to review. :)

Done. ;)

[nolouch]

lgtm

[breezewish]

I'm wondering do we have an official place that mentions that we should follow uber's go style? Otherwise there are multiple styles in the wild and it is not suitable to apply rules from different styles together.

[tisonkun]

I second @breeswish and suspect that if we don't have a style checker, this change is meaningless and unmaintainable.

It's totally OK to follow some codestyle we don't forbid when writing new code or refactor, but please prevent from do refactor for formatting without guard only - it causes noise.

[tisonkun]

an official place that mentions that we should follow uber's go style

No. We've made a style guide on dev guide (later we simplify it) and the original context includes:

Make sure the PR follows our code style and quality guide. For Go and Rust, there are built-in tools with the compiler toolchain.

If you want to improve styles not covered in the style guide, you can comment that you think it would improve the code readability but it is not mandatory. Do not block PRs from being merged based only on personal style preferences.

The PR author should not combine nontrivial style changes and functional changes in one PR, because it violates the guideline covered earlier: one PR does one thing.

The simplify (and current) version is:

Style. Code in the pull request should follow common programming style. For Go and Rust, there are built-in tools with the compiler toolchain. However, sometimes the existing code is inconsistent with the style guide, you should maintain consistency with the existing code or file a new issue to fix the existing code style first.

I think it means that, if the style is not documented and/or guarded by CI, it's unnecessary and suspicious to force to follow.

[disksing]

IMO any uncontroversial and positive improvement to the code should be allowed to merge, regardless of whether it's a PR on its own or with other refactorings, or whether there are rules that specify the style of code we need to follow.

[tisonkun]

uncontroversial

Here is obviously controversies now.

positive improvement
whether there are rules that specify the style of code we need to follow

It's suspicious. "Functions should be sorted in rough call order.", what is "rough"?

To be clear, I totally agree with follow style you like and not forbidden when writing new code or non style-only refactor. If you like a style and want to apply in the codebase,

If you want to improve styles not covered in the style guide, you can comment that you think it would improve the code readability but it is not mandatory.

I'm afraid that we open the gate to random changes which is hardly proven to be good or converged. These changes increase the difficulty to do cherry-pick without non-trivial benefits.

• Functions should be sorted in rough call order.
• Functions in a file should be grouped by receiver.

They're good to follow when writing new code, and if you want to spread in the community, start a thread to add it to our style guide - but still, it will apply on new code in recommend without a checker. Also reject such style-only changes without checker or official rules to follow save our bandwidth.