お知らせ

現在サイトのリニューアル作業中のため、表示が崩れているページが存在することがあります。

シンプルで事故が起き辛いGitのブランチフローを考えて実務で運用した話

投稿日：ソフトウェア::Git開発::設計開発::自動化

これは、かつて参画したGitHubを利用したプロジェクトでブランチフローが悪く事故が多発したので考案し、運用した内容です。
基本的には後々の運用を考えた時に情報源になり、かつGit操作に極力手間を取られることがなく、CI/CDを回しながら品質を維持できる内容で考えています。

フローの要件として考えたこと

取り回しが単純明快であること

開発以外の要素に振り回されないように単純なフローにしていて、世に言われる履歴の綺麗さとか言うのは個人的には関心が薄いので重視していません。その代わりコミットが壊れないことや、インデックスとして見やすくなるような部分を重視しています。

GitHubとの相性が良いこと

まず個々の開発タスクをIssueベースで管理し、Pull Requestベースで取り込む運用としました。
Pull Requestの取り込み方式はSquash Mergeとし、メインブランチのコミット履歴がPull Requestのマージコミットになるようにしました。
これはメインブランチのコミット履歴はPull Requestのマージコミットだけあれば後から追えるというのと、メインブランチのコミットログを単純にする意味でこの方式にしています。

CI/CDを活用しやすいこと

これは割とどこでもやっていると思いますが、ブランチ名にdevelopとかstaging, productionとか付けて管理することで、自動的に環境を識別できるようにしました。

実際に運用したフロー

フロー図

運用したブランチフロー

フローの運用内容

  • develop/mainブランチを最新ブランチとする運用
    • develop/mainブランチ相当のものが複数ある状態というのが世の中にあると思いますが、管理が非常に大変なのでそれはしない方向にしました
  • 機能ブランチをdevelop/mainブランチに取り込むのはSquash Merge
    • 基本的に変更履歴を見る時はGitLensやblameで変更行からPull Requestを当てて、そこを見に行くという運用にしていました
  • 機能ブランチにdevelop/mainブランチを取り込むのはmerge
    • 一般的にはrebaseが多いと思いますが、次の観点から採用しませんでした
      • どのポイントで取り込んだのかわからない
      • mergeと比較した場合にコンフリクト対応に手を取られる
        • 経験上ここで事故が頻発する
      • 素直にpush出来ない
    • mergeであれば以下のように単純な流れに出来ますし、push前に差分確認して事故を防ぐことも容易です
      • git switch develop/main
      • git pull
      • git switch -
      • git merge -
      • コンフリクトがあれば解消
      • git push
  • デプロイ方式によるルートブランチ分割
    • ルートブランチ名によってデプロイ先を変更できるようにしました
    • GitHub Actionsでブランチ名を拾って環境変数を差し替えることでデプロイするワークフローを組んでいます
    • staging/やproduction/ブランチはdevelop/mainからcheckoutする運用です
      • これらはデプロイするためだけの使い捨てブランチなので毎回生えます

あとがき

このフローの利点としてはマージでコンフリクトが起きても基本的にCurrentとIncomingを一回比較するだけで済むのでコンフリクトの解消が簡単で、コンフリクト時はIncomingが壊れないようにCurrentを直すのが基本になり、Currentを優先する場合は適宜上書きするといった内容です。

ブランチの合流はmergeだとマージコミット分一回の解消だけでいいので事故の発生要因が低いのがrebaseと比較した時の利点です。rebaseだとコミット回数分再帰的に合流させる必要があるのでブランチの寿命が長かったりすると苦行になってきます。

このフローができた経緯としては元々はGitHub Flowをベースにしていたのですが、色々やっていくうちにこうしたらもっと良くなるのではないか？というのを試行錯誤していてこの形に落ち着いたのですが、後から調べたらGitLab Flowに近い形式に見えたので、似た内容は既に誰かが考えているものだなと感じました。

上で挙げた内容の他にもGitHubのリポジトリ設定でブランチ保護のルールを設定したり、Pull Requestのマージ設定でSquash mergingだけ許可したり、ヘッドブランチの自動削除をするなど、基本的に面倒なことを考えたり、しなくて良い様にするなど、開発に注力しやすいように環境を整えると心理的な抵抗が少なく運用できて便利だなと感じています。

コメント（0件）

Reactでより良い開発をするために意識したいこと

投稿日：ライブラリ::React開発::設計

別にReactに限った話でもないのですが、実務で悩ましいコードにあたって頭を抱えてる内にハゲてきたので、ハゲがこれ以上進まない事を祈り書きました。いやいっそのこと全ハゲになりたいですが、それはさておき
とりあえず個人的には以下2点を意識すれば起きないとは思うので、Next.jsを使った簡単なサンプルを交えながら3例ほどケーススタディ形式で紹介していきます

  • 密結合にしない
  • シンプルに実装する

挙げている事例については例示用にフルスクラッチで書いています（主題と関係ないコードは端折ってます
ここはこうした方がより良いのでは？などのご意見があれば是非コメントいただけると嬉しいです

コンポーネントのOptional引数のオーバーライド

コンポーネント引数をコンポーネント側で書き換えるような実装はどうかなと思います

問題点

  • 親の預かり知らぬところで値が設定されている
    • 誰かが知らずに設定値をすり替えたりしたら不具合が起きそうです
  • 型のコメントと設定値が異なる
    • 型のコメントと実装が異なるので、誰かが直すかもしれません
      • すると、コンポーネントを参照している全コンポーネントに影響が波及します
    • そもそも型と実装は本質的に関係ないので、こういった運用はNG

一例

type AccountContainerProps = {
  email: string;
  // デフォルトtrue
  isUpdate?: boolean;
  // デフォルトfalse
  gotNotify?: boolean;
};


export const AccountContainer = ({
  isUpdate = true,
  gotNotify = true,
  ...props
}: AccountContainerProps) => {
  ...
}

改善案

呼び元で明示的に指定するように変更しています
これによって親は子の実装を知る必要がなくなり、責務がそこで閉じるようになりました

改善点

  • 親の預かり知らぬところで値が設定されなくなった
    • これで子コンポーネントで値が変わることに怯える必要はなくなりました
  • 型の初期値コメントを削除できた
    • 実装と一致する保証がないコメントは削除するべきでしょう
type AccountContainerProps = {
  email: string;
  isUpdate: boolean;
  gotNotify: boolean;
};


export const AccountContainer = (props: AccountContainerProps) => {
  ...
}

コンポーネントのOptional引数の多重オーバーライド

コンポーネントのOptional引数のオーバーライドが多重化されている上に、なんか途中で更に書き換えられているとかいう地獄
どうしてそんなことをするのか…

一例

type AccountTemplateProps = {
  email: string;
  // デフォルトtrue
  isUpdate?: boolean;
  // デフォルトfalse
  gotNotify?: boolean;
  from: 'register' | 'update';
};

export const AccountTemplate = ({
  isUpdate = false,
  gotNotify = true,
  ...props
}: AccountTemplateProps) => {
  if (props.from === 'register') isUpdate = false;
  return <AccountContainer {...props} />;
};

type AccountContainerProps = {
  email: string;
  // デフォルトtrue
  isUpdate?: boolean;
  // デフォルトfalse
  gotNotify?: boolean;
};


export const AccountContainer = ({
  isUpdate = true,
  gotNotify = true,
  ...props
}: AccountContainerProps) => {
  ...
}

改善案

前項のように明示的に値を渡してあげるようにしましょう

直列に分散されたコンポーネント

コンポーネントの中にコンポーネントがネストされ続けてるパターンです
一見して何をしているのか分かりづらい上、StateやらHookやら色んな処理が各コンポーネントに分散配置されていることもあります

問題点

  • 親からみると子が何をしているのかが分かりづらい
  • コンポーネント名が意味をなしていない（責務が別れていない）
  • 子コンポーネントが状態を持っているため、親コンポーネントでハンドリングができない
  • 子コンポーネントのロジック変更が参照している全コンポーネントのロジックに波及する

一例

親コンポーネント

RegisterHeaderなる物が差し込まれていることだけがわかります
このコンポーネントが何をするのかはパッと見ではよくわかりません

const AccountUpdatePage = () => {
  return <Register header={<RegisterHeader />} />;
};
ラッピングしているコンポーネント

ラッパーなのでこのコンポーネントそのものは何をしているのかわかりません

type RegisterProps = {
  header: JSX.Element;
};

export const Register = (props: RegisterProps) => {
  return <>{props.header}</>;
};
差し込まれているコンポーネントの中身

どうやらRegisterHeaderはRegisterContentを含むようです
なんでヘッダーの中にコンテンツがあるのでしょうか…

export const RegisterHeader = () => {
  return (
    <>
      <p>head</p>
      <RegisterContent />
    </>
  );
};
差し込まれているコンポーネントの子

現在のパスに応じて叩くAPIを変えるような実装がされていますが、もしパスが変わったり増えたりしたらこのコンポーネントの実装を知らない限り面倒なことになります

export const RegisterContent = () => {
  const rt = useRouter();
  const currentPath = rt.pathname;
  const url =
    currentPath === 'register'
      ? 'https://example.com/kaiin/touroku'
      : 'https://example.com/kaiin/koshin';
  const [username, setUsername] = useState<string | undefined>(undefined);

  const onSubmit = (ev: React.FormEvent<HTMLFormElement>) => {
    ev.preventDefault();
    axios.post(url, {
      username,
    });
  };

  return (
    <>
      <form onSubmit={onSubmit}>
        <input
          type="text"
          value={username}
          onChange={(ev) => setUsername(ev.target.value)}
        />
        <button>送信</button>
      </form>
      <RegisterFooter />
    </>
  );
};

export const RegisterFooter = () => {
  return <p>foot</p>;
};

改善案

改善点

  • 親から子への見通しが改善された
  • コンポーネント名が名前の通りの意味を持つようになった
    • ヘッダーはヘッダー、フォームはフォーム、フッターはフッターの責務だけに集中できます
    • Registerとか言う謎コンポーネントも姿を消しました
  • 状態をすべて親に集約した
  • 子コンポーネントのロジック変更が起きる確率が減った
    • 子コンポーネント側のロジックを減らし、親から渡されるコールバックで行うようにしたため、子コンポーネントのロジック変更が他に影響する確率が減りました
親コンポーネント
  • ひとまずヘッダーと入力フォーム、フッターがあるんだなという事が解るようにはなったと思います
  • 状態を親に集約したのでAPI叩く時のURIも態々パスから判断しなくて良くなったのでコードの複雑性が減っています
const usePageState = () => {
  const [username, setUsername] = useState('');

  return {
    username,
    setUsername,
  };
};

const onSubmit = (username: string) => {
  axios.post('https://example.com/kaiin/koshin', {
    username,
  });
};

const AccountUpdatePage = () => {
  const ps = usePageState();

  return (
    <>
      <RegisterHeader />
      <RegisterForm
        onChangeUsername={ps.setUsername}
        username={ps.username}
        onSubmit={() => onSubmit(ps.username)}
      />
      <RegisterFooter />
    </>
  );
};
ヘッダー
export const RegisterHeader = () => {
  return <p>head</p>;
};
入力フォーム
type RegisterFormProps = {
  username: string;
  onChangeUsername: (value: string) => void;
  onSubmit: () => void;
};

const onChangeUsername = (
  ev: React.ChangeEvent<HTMLInputElement>,
  setState: (value: string) => void
) => {
  const value = ev.target.value;
  setState(value);
};

const onSubmit = (
  ev: React.FormEvent<HTMLFormElement>,
  onSubmit: () => void
) => {
  ev.preventDefault();
  onSubmit();
};

export const RegisterForm = (props: RegisterFormProps) => {
  return (
    <>
      <form onSubmit={(ev) => onSubmit(ev, props.onSubmit)}>
        <input
          type="text"
          value={props.username}
          onChange={(ev) => onChangeUsername(ev, props.onChangeUsername)}
        />
        <button>送信</button>
      </form>
    </>
  );
};
フッター
export const RegisterFooter = () => {
  return <p>foot</p>;
};
コメント（0件）