5 篇博文 含有标签「Engineering Practices」

TD;DR: 配置在这里: https://github.com/qiniu/reviewbot/blob/main/.golangci.yml

为什么会有这份配置呢？​

提到 go 领域的静态检查，除了 go 官方提供的 go vet 之外，大家一般都会选择使用 golangci-lint，因为这个工具集合了当前 go 领域的几乎所有主流的 lint 工具。且使用也相对简单。不光在命令行场景下可以直接使用，在各种 CI 场景中，也都有方便的支持。

但就像我在之前文章中提到的，golangci-lint 的配置项非常多，且很多配置项的默认值，并不是最佳实践。所以，在实际使用中，大家往往需要根据自身项目的情况，进行针对性的配置。

但当你想启用更多的 linter 时，你可能随之会发现 "golangci-lint 官方" 好像并没有给出一份最佳实践的配置。

咦，这是为什么呢？

估计还是定位原因，不能有太过明确的倾向，不然其他的 linter 作者哪还有动力往 golangci-lint 里添加新的 linter，对吧?

当然，不同的项目，其关注点也会不同，比较难有通用的最佳实践。

不过呢, qiniu/reviewbot 这边，经过调研和实践后，还是总结出了一份配置，当然仅供参考。

这份配置的严肃性​

这份配置是怎么来的呢？

• 首先参考了一个关注比较多的配置。

https://gist.github.com/maratori/47a4d00457a92aa426dbd48a18776322

• 然后，我将 golangci-lint 集成的所有 linter，按其 star 数排序，重点选取了 100+ star 的 linter，并参考了其配置。

• 然后又参考了几个著名公司/项目的配置。

https://github.com/golangci/golangci-lint/blob/master/.golangci.yml

• 最后，就是基于七牛内部实践，形成的一份配置。

如此，这份配置就形成了。

当然，这份配置并不适合所有项目，但它可以作为你项目的初始配置，然后根据项目的实际情况，进行必要调整。

感谢阅读。

Reviewbot 是七牛云开源的一个项目，旨在提供一个自托管的代码审查服务, 方便做 code review/静态检查, 以及自定义工程规范的落地。

自从上了 Reviewbot 之后，我发现有些 lint 错误，还是很容易出现的。比如

dao/files_dao.go:119:2: `if state.Valid() || !start.IsZero() || !end.IsZero()` has complex nested blocks (complexity: 6) (nestif)

cognitive complexity 33 of func (*GitlabProvider).Report is high (> 30) (gocognit)

这两个检查，都是圈复杂度相关。

圈复杂度（Cyclomatic complexity）是由 Thomas McCabe 提出的一种度量代码复杂性的指标，用于计算程序中线性独立路径的数量。它通过统计程序控制流中的判定节点（如 if、for、while、switch、&&、|| 等）来计算。圈复杂度越高，表示代码路径越多，测试和维护的难度也就越大。

圈复杂度高的代码，往往意味着代码的可读性和可维护性差，非常容易出 bug。

为什么这么说呢？其实就跟人脑处理信息一样，一件事情弯弯曲曲十八绕，当然容易让人晕。

所以从工程实践角度，我们希望代码的圈复杂度不能太高，毕竟绝大部分代码不是一次性的，是需要人来维护的。

那该怎么做呢？

这里我首先推荐一个简单有效的方法：Early return。

Early return - 逻辑展平，减少嵌套​

Early return, 也就是提前返回，是我个人认为最简单，日常很多新手同学容易忽视的方法。

举个例子：

func validate(data *Data) error {
    if data != nil {
        if data.Field != "" {
            if checkField(data.Field) {
                return nil
            }
        }
    }
    return errors.New("invalid data")
}

这段代码的逻辑应该挺简单的，但嵌套层级有点多，如果以后再复杂一点，就容易出错。

这种情况就可以使用 early return 模式改写，把这个嵌套展平：

func validate(data *Data) error {
    if data == nil {
        return errors.New("data is nil")
    }
    if data.Field == "" {
        return errors.New("field is empty")
    }
    if !checkField(data.Field) {
        return errors.New("field validation failed")
    }
    return nil
}

是不是清晰很多，看着舒服多了？

记住这里的诀窍：如果你觉得顺向思维写出的代码有点绕，且嵌套过多的话，就可以考虑使用 early return 来反向展平。

当然，严格意义上讲，early return 只能算是一种小技巧。要想写出高质量的代码，最重要的还是理解 分层、组合、单一职责、高内聚低耦合、SOLID 原则等 这些核心设计理念 和 设计模式了。

Functional Options 模式 - 参数解耦​

来看一个场景: 方法参数很多，怎么办？

比如这种：

func (s *Service) DoSomething(ctx context.Context, a, b, c, d int) error {
    // ...
}

有一堆参数，而且还是同类型的。如果在调用时，一不小心写错了参数位置，就很麻烦，因为编译器并不能检查出来。

当然，即使不是同类型的，参数多了可能看着也不舒服。

怎么解决？

这种情况，可以选择将参数封装成一个结构体，这样在使用时就会方便很多。封装成结构体后还有一个好处，就是以后增删参数时（结构体的属性），方法签名不需要修改。避免了以前需要改方法签名时，调用方也需要跟着到处改的麻烦。

不过，在 Go 语言中，还有一种更优雅的解决方案，那就是Functional Options 模式。

不管是 Rob Pike 还是 Dave Cheney 以及 uber 的 go guides 中都有专门的推荐。

• https://commandcenter.blogspot.com/2014/01/self-referential-functions-and-design.html
• https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis
• https://github.com/uber-go/guide/blob/master/style.md#functional-options

这种模式，本质上就是利用了闭包的特性，将参数封装成一个匿名函数，有诸多妙用。

Reviewbot 自身的代码中，就有相关的使用场景(https://github.com/qiniu/reviewbot/blob/c354fde07c5d8e4a51ddc8d763a2fac53c3e13f6/internal/lint/providergithub.go#L263)，比如：

// GithubProviderOption allows customizing the provider creation.
type GithubProviderOption func(*GithubProvider)
func NewGithubProvider(ctx context.Context, githubClient *github.Client, pullRequestEvent github.PullRequestEvent, options ...GithubProviderOption) (*GithubProvider, error) {
    // ...
    for _, option := range options {
        option(p)
    }
    // ...
    if p.PullRequestChangedFiles == nil {
        // call github api to get changed files
    }
    // ...
}

这里的 options 就是 functional options 模式，可以灵活地传入不同的参数。

当时之所以选择这种写法，一个重要的原因是方便单测书写。

为什么这么说呢？

看上述代码能知道，它需要调用 github api 去获取 changed files， 这种实际依赖外部的场景，在单测时就很麻烦。但是，我们用了 functional options 模式之后，就可以通过 p.PullRequestChangedFiles 是否为 nil 这个条件，灵活的绕过这个问题。

Functional Options 模式的优点还有很多，总结来讲(from dave.cheney):

• Functional options let you write APIs that can grow over time.
• They enable the default use case to be the simplest.
• They provide meaningful configuration parameters.
• Finally they give you access to the entire power of the language to initialize complex values.

现在大模型相关的代码，能看到很多 functional options 的影子。比如 https://github.com/tmc/langchaingo/blob/238d1c713de3ca983e8f6066af6b9080c9b0e088/llms/ollama/options.go#L25

type Option func(*options)
// WithModel Set the model to use.
func WithModel(model string) Option {
	return func(opts *options) {
		opts.model = model
	}
}
// WithFormat Sets the Ollama output format (currently Ollama only supports "json").
func WithFormat(format string) Option {
    // ...
}
//	If not set, the model will stay loaded for 5 minutes by default
func WithKeepAlive(keepAlive string) Option {
    // ...
}

所以建议大家在日常写代码时，也多有意识的尝试下。

善用 Builder 模式/策略模式/工厂模式，消弭复杂 if-else​

Reviewbot 目前已支持两种 provider(github 和 gitlab)，以后可能还会支持更多。

而因为不同的 Provider 其鉴权方式还可能不一样，比如:

• github 目前支持 Github APP 和 Personal Access Token 两种方式
• gitlab 目前仅支持 Personal Access Token 方式

当然，还有 OAuth2 方式，后面 reviewbot 也也会考虑支持。

那这里就有一个问题，比如在 clone 代码时，该使用哪种方式？代码该怎么写？使用 token 的话，还有个 token 过期/刷新的问题，等等。

如果使用 if-else 模式来实现，代码就会变得很复杂，可读性较差。类似这种：

if provider == "github" {
    // 使用 Github APP 方式
    if githubClient.AppID != "" && githubClient.AppPrivateKey != "" {
        // 使用 Github APP 方式
        // 可能需要调用 github api 获取 token
    } else if githubClient.PersonalAccessToken != "" {
        // 使用 Personal Access Token 方式
        // 可能需要调用 github api 获取 token
    } else {
        return err
    }
} else if provider == "gitlab" {
    // 使用 Personal Access Token 方式
    if gitlabClient.PersonalAccessToken != "" {
        // 使用 Personal Access Token 方式
        // 可能需要调用 gitlab api 获取 token
    } else {
        return errors.New("gitlab personal access token is required")
    }
}

但现在 Reviewbot 的代码中，相关代码仅两行:

func (s *Server) handleSingleRef(ctx context.Context, ref config.Refs, org, repo string, platform config.Platform, installationID int64, num int, provider lint.Provider) error {
    // ...
	gb := s.newGitConfigBuilder(ref.Org, ref.Repo, platform, installationID, provider)
	if err := gb.configureGitAuth(&opt); err != nil {
		return fmt.Errorf("failed to configure git auth: %w", err)
	}
	// ...
}

怎么做到的呢？

其实是使用了 builder 模式，将 git 的配置和创建过程封装成一个 builder，然后根据不同的 provider 选择不同的 builder，从而消弭了复杂的 if-else 逻辑。

当然内部细节还很多，不过核心思想都是将复杂的逻辑封装起来，在主交互逻辑中，只暴露简单的使用接口，这样代码的可读性和可维护性就会大大提高。

最后​

到底如何写出高质量的代码呢？这可能是很多有追求的工程师，一直在思考的问题。

在我看来，可能是没有标准答案的。不过呢，知道一些技巧，并能在实战中灵活运用，总归是好的。

你说是吧？

Reviewbot 是七牛云开源的一个项目，旨在提供一个自托管的代码审查服务, 方便做 code review/静态检查, 以及自定义工程规范的落地。

在日常的编程协作中，Git commit 记录的质量往往反映了一个工程师的工程素养。然而，我经常能看到一些不太规范的 commit 记录。有时，真的不敢恭维。

比如这种:

这种大概率是提交 commit 之后，又有变动，就随手重新复用上一条 git commit 命令了。

这种记录如果出现在个人仓库，可能还好. 但如果是多人协作的仓库，就有点不专业了。

在我看来，这些 commit 记录完全没必要，是非常不好的习惯，完全可以避免。

好在 Git 为我们提供了优雅的解决方案。如果没必要生成新的 commit，那直接使用 git commit --amend 就可以避免。

少用 git merge 多用 git rebase​

比如这种:

Merge branch 'feature-A' of https://github.com/qiniu/reviewbot into feature-B

说的是把远程分支 feature-A 的代码合并到 feature-B 里。这里的 feature-A 通常是主分支。

这种 Commit 信息如果出现在你的 PR 里，那是完全没必要。PR 里的 commit 信息应当仅包含针对本次改动的有用信息。

我个人日常几乎不使用 git merge，即使是为了同步远程分支，我一般都会使用 git rebase。

比如:

git rebase 除了上述好处外，还可以保持主仓库的 commit history 非常干净。所以强烈推荐大家使用。

Reviewbot 的 git commit check​

为了更好的规范上述两种行为，Reviewbot 也添加了 git commit check 能力，就是用来检查 git commit 记录是否符合规范的。

如果不符合规范，Reviewbot 就会提示你:

更多 git flow 使用规范和技巧​

当然 git 操作其实有很多实用技巧，建议大家有兴趣的话可以去研究下。我在 1024 实训营的时候，有给同学们做个相关分享:

超实用! 从使用视角的 Git 协作实战，告别死记硬背 。

文档里面有视频链接，感兴趣的同学可以去看下。

最后，作为专业的工程师，我们应该始终追求卓越的工程实践。良好的 commit 记录不仅体现了个人的专业素养，更是提升团队协作效率的重要基石。

通过合理使用 git rebase 和 git commit --amend，我们可以维护一个更清晰、更专业的代码提交历史。这不仅让代码审查变得更加轻松，也为后续的代码维护和问题追踪带来极大便利。

你觉得呢？

Reviewbot 是七牛云开源的一个项目，旨在提供一个自托管的代码审查服务, 方便做 code review/静态检查, 以及自定义工程规范的落地。

静态检查不是个新鲜事。

我记得早在几年前，我们就调研并使用过 sonarqube 做静态检查，但当时并没有大范围的推广。主要原因在于，一是发现的问题多数是风格问题，较少能发现缺陷; 二是 sonarqube 社区版的 worker 数有限制，满足不了我们大规模代码扫描的需求。当然，也是因为前一个问题，感觉付费并不是很划算。

而由于七牛主要使用 golang 语言，所以在静态检查方面，我们基本使用 go vet/fmt/lint 等，再就是后来的 golangci-lint，好像也够用了。

但随着代码仓库的增多，以及对工程规范的不断强化，我们越来越发现当前的落地方式，已经开始无法满足我们的需求。

Linter 工具的引入与更新问题​

以 golangci-lint 为例，它是 go 语言 linters 聚合器，内置了 100+ linters，使用也很简单, golangci-lint run 一条命令即可。但你知道吗？如果没有特殊配置，你这条命令其实仅仅执行其中的 6 个 linter，绝大部分 linters 都没有执行！

另外，工具本身需要更新，且很多时候我们也会自己定制 linter 工具，这种时候该怎么做呢？如果仅有少量仓库，可能还好，但如果仓库很多，那维护成本就上去了。

还有就是新业务，新仓库，如何保证相关的检查能够及时配置，相关的规范能够正确落地？

靠自觉一定是不行的。

Linter 问题的发现与修复情况​

如何确保发现的问题能够被及时修复？如何让问题能更及时、更容易的被修复？

埋藏在大量 raw log 中的问题，一定是不容易被发现的，查找起来很麻烦，体验很差。

历史代码仓库的存量问题，谁来改？改动就需要时间，但实际上很多业务研发可能并没有动力来跟进。同样，变动总是有风险的，有些 lint 问题修复起来也并不简单，如果因修复 lint 问题而引入风险，那就得不偿失了。

如果想了解当前组织内 lint 问题的分布及修复情况，又该怎么办呢？

如何解决，方向在哪里？​

不可否认，linter 问题也是问题，如果每行代码都能进行充分的 lint 检查，那一定比不检查要强。

另一方面，组织内制定好的工程规范，落地在日常的开发流程中，那一定是希望被遵守的，这类就是强需。

所以这个事情值得做，但做的方式是值得思考的，尤其是当我们有更高追求时。

参考 CodeCov 的服务方式，以及 golangci-lint reviewdog 等工具的设计理念，我们认为:

• 如果能对于新增仓库、历史仓库，不需要专人配置 job，就能自动生效，那一定是优雅的
• 如果能只针对 PR/MR 中的变动做分析和反馈，类似我们做 Code Review 那样，那对于提 PR 的同学来讲一定是优雅的，可接受的，随手修复的可能性极大
  • 而进一步，针对 PR/MR 中涉及的文件中的历史代码进行反馈，在合理推动下，支持夹带修改，持续改进的可能性也会大大增强
• Lint 工具多种多样，或者我们自己开发出新工具时，能够较为轻松的让所有仓库都自动生效，那也一定是非常赞的，不然就可能陷入工具越多负担越重的风险

基于上面的思考，我认为我们需要的是: 一个中心化的 Code Review/静态检查服务，它能自动接受整个组织内 PR/MR 事件，然后执行各种预定义的检查，并给与精确到变动代码行的有效反馈。它要能作为代码门禁，持续的保障入库代码质量。

而 Reviewbot 就是这样一个项目。

Reviewbot 在设计和实现上有哪些特点？​

面向改进的反馈方式

这将是 Reviewbot 反馈问题的核心方式，它会尽可能充分利用各 Git 平台的自身能力，精确到变动的代码行，提供最佳的反馈体验。

• Github Check Run (Annotations)
• Github Pull Request Review (Comments)

支持多种 Runner

Reviewbot 是自托管的服务，推荐大家在企业内自行部署，这样对私有代码更友好。

Reviewbot 自身更像个管理服务，不限制部署方式。而对于任务的执行，它支持多种 Runner，以满足不同的需求。比如:

• 不同的仓库和 linter 工具，可能需要不同的基础环境，这时候你就可以将相关的环境做成 docker 镜像，直接通过 docker 来执行
• 而当任务较多时，为了执行效率，也可以选择通过 kubernetes 集群来执行任务。

使用也很简单，在配置文件中的目标仓库指定即可。类似:

dockerAsRunner:
  image: "aslan-spock-register.qiniu.io/reviewbot/base:go1.22.3-gocilint.1.59.1"

kubernetesAsRunner:
  image: "aslan-spock-register.qiniu.io/reviewbot/base:go1.23.2-gocilint.1.61.0"
  namespace: "reviewbot"

零配置+定制化

本质上，Reviewbot 也是个 webhook 服务，所以我们只需要在 git provider 平台配置好 Reviewbot 的回调地址即可(github 也可以是 Github App)。

绝大部分的 linter 的默认最佳执行姿势都已经固定到代码中，如无特殊，不需要额外配置就能对所有仓库生效。

而如果仓库需要特殊对待，那就可以通过配置来调整。

类似:

org/repo:
  linters:
    golangci-lint:
      enable: true
      dockerAsRunner:
        image: "aslan-spock-register.qiniu.io/reviewbot/base:go1.22.3-gocilint.1.59.1"
      command:
        - "/bin/sh"
        - "-c"
        - "--"
      args:
        - |
          source env.sh
          export GO111MODULE=auto
          go mod tidy
          golangci-lint run --timeout=10m0s --allow-parallel-runners=true --print-issued-lines=false --out-format=line-number >> $ARTIFACT/lint.log 2>&1

可观察

Reviewbot 是在对工程规范强管理的背景下产生的，那作为工程规范的推动方，我们自然有需求想了解组织内当前规范的执行情况。比如, 都有哪些问题被检出？哪些仓库的问题最多？哪些仓库需要特殊配置？

目前 Reviewbot 支持通过企业微信来接收通知，比如:

• 检出有效问题

• 遇到错误

当然，未来可能也会支持更多方式。

其他更多的功能和姿势，请参考仓库: https://github.com/qiniu/reviewbot

Reviewbot 的未来规划​

作为开源项目，Reviewbot 还需要解决很多可用性和易用性问题，以提升用户体验，比如最典型的，接入更多的 git provider(gitlab/gitee 等)，支持 CLI 模式运行。

但我个人认为，作为 code review 服务，提供更多的检测能力，才是重中之重。因为这不光是行业需求，也是我们自身需要。

所以后面我们除了会引入七牛内部推荐的规范，也会调研和探索更多的行业工具，同时会考虑引入 AI，探索 AI 在 code review 中的应用等等。

Anyway，Reviewbot 还很年轻，我们在持续的改进中，非常欢迎大家试用并提出宝贵意见。当然，更欢迎大家一起参与到项目建设中来。

为了方便沟通，我建了 微信 和 QQ 群，欢迎大家扫码加入，一起交流。

感谢大家。

Looking to build a self-hosted code review service? Try Reviewbot, now open-sourced!

When do you need a self-hosted code review service?​

You might need one when:

• You have many repos but still want tight control over code quality
• Your repos are private, and commercial services seem overkill
• You want to continuously improve the process and rules, with full customization

Benefits of a self-hosted code review service​

While many linter tools and engineering practices exist, they're often underutilized:

• Powerful tools like golangci-lint (with 100+ integrated linters) are often used with default settings, ignoring most features
• Linter outputs get buried in logs, making issue-finding a chore
• Configuring CLI-based linters for multiple repos is tedious, especially for ongoing improvements
• Monitoring code quality across repos can be daunting

A self-hosted service can automate all this. As a DevOps or QA team member, you can easily centralize control, monitoring, and customization of code quality across all repos.

Enter Reviewbot - your solution for self-hosted code review.

What can Reviewbot do?​

Reviewbot helps you quickly set up a self-hosted code analysis and review service, supporting multiple languages and coding standards. It's perfect for organizations with numerous private repos.

Issues are reported during Pull Requests as Review Comments or Github Annotations, pinpointing exact code lines.

• Github Check Run (Annotations)

  

• Github Pull Request Review Comments

  

This approach saves PR authors from sifting through lengthy logs, streamlining problem-solving.

Reviewbot's Design Philosophy​

Focused on:

• Security - Self-hosting for data control
• Improvement-Oriented - Issues reported as Review Comments or Github Annotations for easy fixes
• Flexibility - Multi-language support with easy tool integration
• Observability - Alert notifications for timely issue awareness
• Configurable - Customizable linter commands, parameters, and environments

Built with Golang, Reviewbot boasts simple logic and clear code for easy maintenance.

Main Flow​

Reviewbot primarily operates as a GitHub Webhook/App service, accepting GitHub Events, executing various checks, and providing precise feedback on the corresponding code if issues are detected.

Github Event -> Reviewbot -> Execute Linter -> Provide Feedback

And you can easily Add a New Linter or do Customization.

Monitoring Detection Results​

Reviewbot supports notification of detection results through WeWork (企业微信) alerts.

If unexpected output is encountered, notifications will also be sent, like this:

More​

Check out Reviewbot. Feel free to have a try.

Btw, this article is also published on medium website.

Stay tuned for more updates!