OpenPNE 3

更新履歴 c977c64d39ffdebf58afa173dec21d212a5314c1 で適用されました。

レビューしました。

修正内容としては問題が解消しているためこれで OK としても良いのですが、以下について修正可能であれば対応して頂きたいです。

差し戻し内容¶

2 個目の修正内容について¶

http://redmine.openpne.jp/issues/1872#note-7 では「（3.4 の場合は Member::getAge() 内に条件分岐を追加する）」と示されていますが、

    $publicFlag = $this->getConfig('age_public_flag');
    $publicFlag = null === $publicFlag ? ProfileTable::PUBLIC_FLAG_PRIVATE : $publicFlag;
    if (!$viewableCheck || $publicFlag == ProfileTable::PUBLIC_FLAG_SNS)

この箇所で、このように条件演算子を用いるとこの部分の意味が分かりにくいので、 if 文で記述した方がよいと思います。

    $publicFlag = $this->getConfig('age_public_flag');
    if (null === $publicFlag)
    {
      $publicFlag = ProfileTable::PUBLIC_FLAG_PRIVATE;
    }
    if (!$viewableCheck || $publicFlag == ProfileTable::PUBLIC_FLAG_SNS)

3 個目の修正が取り込まれていません¶

親チケットでは 3 個のコミットが行なわれていますが、このチケットでは 3 個目のコミット（コーディング規約違反を修正する内容）が取り込まれていません。取り込む必要がないと判断したのであればそれでもよいですが、コミットを見逃している可能性があるので確認しておきます。参考 http://redmine.openpne.jp/issues/1872#note-19

上記の修正を行うか、修正する必要がないと判断した旨をコメントするかの対応をお願いします。

更新履歴 ce11fcb0dd545f117bc9a771233a8e7fd9bbc85f で適用されました。

note-6, note-7 での取り込みをレビューしました。

親チケット 3 個目のコミットの取り込みについて、親チケットでは 3 箇所が修正されていますが、本チケットでは 2 箇所しか修正されておらずコンフリクトが生じています。しかしながら、これは 3 箇所目の修正部分が #667 でのコミット c615f5d4 (3.5.0 時点) で追加されたものであるためであり、取り込みは問題ないと判断できます。

親チケット 2 個目のコミットで、条件演算子（三項演算子）ではなく if 文を用いたほうがよいという指摘に対する修正について、修正が適切に行なわれていると判断できます。

ちなみに note-6 のコミットメッセージを拝見しましたが、一般的に expr ? val : val の演算子は「条件演算子」または「三項演算子（条件演算子以外に三項の演算子がないため）」と呼ばれており、英語では Conditional Operator （条件演算子）または Ternary Operator （三項演算子 : see PHP Manual ）と表記されます。

レビュー OK とします。