從一個重構項目中能學到什麼東西

本來這篇文章是要在 2022 最後一個工作日前寫完的，但是拖延癌發作，到現在才寫完。不過還是發出來，希望裡面的內容能幫到大家

背景介紹#

這個重構專案如果從我第一個超大型重構 PR 算起（22 年 12 月 11 日），到現在已經歷史一個半月了。目前重構進度已經超過了 80%，超過 6 + 位貢獻者集體貢獻。這絕對是個不小的工程了

那問題來了，我為什麼要發起這個重構專案呢？

在重構專案之前，nerdctl 專案存在一個很大的問題，即 command 的入口處，flag 的處理和邏輯耦合的問題，比如用 nerdctl apparmor 系列的代碼來舉一個例子

package main

import (
	"bytes"
	"errors"
	"fmt"
	"text/tabwriter"
	"text/template"

	"github.com/containerd/nerdctl/pkg/apparmorutil"
	"github.com/spf13/cobra"
)

func newApparmorLsCommand() *cobra.Command {
	cmd := &cobra.Command{
		Use:           "ls",
		Aliases:       []string{"list"},
		Short:         "列出已加載的 AppArmor 配置檔",
		Args:          cobra.NoArgs,
		RunE:          apparmorLsAction,
		SilenceUsage:  true,
		SilenceErrors: true,
	}
	cmd.Flags().BoolP("quiet", "q", false, "僅顯示配置檔名稱")
	// Alias "-f" is reserved for "--filter"
	cmd.Flags().String("format", "", "使用給定的 go 模板格式化輸出")
	cmd.RegisterFlagCompletionFunc("format", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
		return []string{"json", "table", "wide"}, cobra.ShellCompDirectiveNoFileComp
	})
	return cmd
}

func apparmorLsAction(cmd *cobra.Command, args []string) error {
	quiet, err := cmd.Flags().GetBool("quiet")
	if err != nil {
		return err
	}
	w := cmd.OutOrStdout()
	var tmpl *template.Template
	format, err := cmd.Flags().GetString("format")
	if err != nil {
		return err
	}
	switch format {
	case "", "table", "wide":
		w = tabwriter.NewWriter(cmd.OutOrStdout(), 4, 8, 4, ' ', 0)
		if !quiet {
			fmt.Fprintln(w, "NAME\tMODE")
		}
	case "raw":
		return errors.New("不支持的格式: \"raw\"")
	default:
		if quiet {
			return errors.New("格式和安靜模式不能同時指定")
		}
		var err error
		tmpl, err = parseTemplate(format)
		if err != nil {
			return err
		}
	}

	profiles, err := apparmorutil.Profiles()
	if err != nil {
		return err
	}

	for _, f := range profiles {
		if tmpl != nil {
			var b bytes.Buffer
			if err := tmpl.Execute(&b, f); err != nil {
				return err
			}
			if _, err = fmt.Fprintf(w, b.String()+"\n"); err != nil {
				return err
			}
		} else if quiet {
			fmt.Fprintln(w, f.Name)
		} else {
			fmt.Fprintf(w, "%s\t%s\n", f.Name, f.Mode)
		}
	}
	if f, ok := w.(Flusher); ok {
		return f.Flush()
	}
	return nil
}

你能看到在函數 apparmorLsAction 的邏輯中包含了兩部分的東西

flag 的處理（大道至簡的 err 處理（XDDDDD
command logic 的處理

這樣的設計存在很明顯的問題

代碼可讀性與可維護性的问题，比如我需要添加一個 flag 的時候，那麼需要在多處添加。而且滿天飛的 flagging process 會導致提升新人進入專案的門檻
logic 的處理與 flag 的處理耦合在一起，這樣會額外導致如果社區在試圖基於 nerdctl 封裝一套自定義的 CLI 腳手架的時候，那麼會出現非常難處理的情況。

同時 nerdctl 還存在另外一個問題。在 cmd 的入口處，因為同歸屬於一個 sub package，於是之前的開發過程中為了省事，文件之間為了省事，交叉引用了彼此的 internal helper function

在 nerdctl 專案最開始只作為 containerd CLI 的一個替代品的時候。之前的設計缺陷實際上暴露的並不明顯。但是 nerdctl 完整提供了一套基於 containerd 的容器生命週期及網路管理（base on CNI）及其餘進階特性（比如 cosign，IPFS 等），開始作為 containerd 實質上的一個入口標準的時候。社區無疑會提出更高的需求。比如 Move *.go files for subcommand out main package nerdctl#1631 就是一個很典型的例子。

在這種情況下，對於 nerdctl 的入口進行一個合理的但是大範圍的重構，就是一個必須且迫在眉睫的事了。

又到了~~白色相薄~~重構的季節 --- 蠻久抚子（Nadeshiko Manju）

重構過程分析#

好了，社區有需要，saka 哦不，蠻久抚子（Nadeshiko Manju）我就得站出來了，重構嘛，很簡單嘛，Goland 搞一搞就完事了嘛。好說好說。於是我有了一個超大的 PR ：Refactor the package structure in cmd/nerdctl nerdctl#1639。規模 +5000 -4000

よし、気合いが勝っとる！

不過，因為這個 PR 太過於驚世駭俗，在我 COVID-19 Positive 後，Suda 開始幫我 carry 這個 PR。但是最後 Suda 也高呼不可 carry（Suda の驚く：ばか saka！）

どうしてこうなるんだろう… 初めて、リファクタリングしたいという欲求があり、リファクタリングの必要性がありました。嬉しいことが二つ重なって。その二つの嬉しさが、また、たくさんの嬉しさを連れてきてくれて。夢のように幸せな時間を手に入れたはずなのに… なのに、どうして、こうなっちゃうんだろう…
為什麼會變成這樣呢，第一次有了想重構的欲望，又有了重構的必要。兩件快樂事情重合在一起。而這兩份快樂，又給我帶來更多的快樂。得到的，本該是像夢境一般幸福的時間…… 但是，為什麼，會變成這樣呢…… ---- 《nerdctl 相薄》

實際上原因很簡單 ~~冬馬小三~~ ，哦不是，是我小三，哦，不是，是我腦子被門夾了

言歸正傳，其實這個 PR 是個教科書式的反面例子

在啟動大型專案之前沒有達成社區的共識
違背了 One PR for One Thing 的基本原則
重構時的無關的改動太多，導致 review 難度過大

所以在吸取了 Refactor the package structure in cmd/nerdctl nerdctl#1639 的教訓後，我正式在社區提出了一個重構 Proposal Let's refactor the nerdctl CLI package nerdctl#1680，在這個 Proposal 中我做了幾個事情

完整闡述了重構的必要性，方便社區成員後續回溯
定義了重構的幾個 step
約定好了多人協作重構時所共同遵守的約定

社區其餘幾位 maintainer 在這個 Proposal 下額外討論了一些細節，並達成了一些共識

將最終的重構範圍縮小為僅處理 flagging process
優化了一些文件結構的設計

截止到現在，nerdctl 的重構才算開始正式進入了一個快車道的狀態。畢竟重構不是亂寫，要是寫錯了，要向社區謝罪的。

這裡面其實還有個插曲，最開始我在 Issue 中創建 TODO Task 之後，為了方便 track project 的進度，我將這些 TODO Task 直接全部轉成了 Issue（然後就相當於給 subscribe 了這個 repo 的老哥們來了一個郵箱 DDOS）。這裡不得不吐槽一句，GitHub 的專案管理工具真的很弱誒（XDDDDD

花開兩朵，各表一枝，在 Proposal 正式通過了之後，整體的重構就開始進入了快車道了，這裡列一些有意思的討論，大家有興趣可以去看看

Refactor the apparmor flagging process nerdctl#1774，Proposal 接收後的一個模板 PR，在這個 PR 下，繼續細化了一些在 Proposal 中討論沒有完善的細節
[Refactor] Refactor the build subcommand flagging process nerdctl#1792，Proposal 接收後第一個比較大命令的重構，某種意義上也是一個模板 PR 了，裡面就討論了不少參數設計風格的問題
refactor: consolidate main logic of volume.List into volume.Volumes, 不屬於 Proposal 原本涵蓋的範圍內，但是裡面關於函數語義設計的討論值得關注一下
pkg/cmd: inconsistent arguments ordering nerdctl#1889，關於函數設計風格的問題。

當然還有很多 PR 中的討論也是非常有意思的，這裡就不完整列出來了。歡迎大家去直接看原始的 PR（當然歡迎加入討論）

總結#

差不多就這樣吧，大概復盤了一下到現在為止重構過程中的得失。希望大家能喜歡