私が歌川です

@utgwkk が書いている

GitHub Actionsのworkflow commandは標準出力と標準エラー出力のどちらに出力するとよいか?

tl;dr

2020/10/13時点 (GitHub Actions runner v2.273.5) では、標準出力 (STDOUT) と標準エラー出力 (STDERR) のどちらに出力してもよい

モチベーション

実験

以下のようなworkflow YAMLを GitHub - utgwkk/sandbox-actions-stdout-stderr に用意して実験した。

name: test
on:
  push:

jobs:
  test:
    name: annotate
    runs-on: ubuntu-latest
    steps:
    - name: Issue error command with STDOUT
      run: python3 -c "print('::error file=.github/workflows/test.yml,line=1::from stdout')"

    - name: Issue error command with STDERR
      run: python3 -c "import sys; print('::error file=.github/workflows/test.yml,line=1::from stderr', file=sys.stderr)"

実験結果

Python · utgwkk/sandbox-actions-stdout-stderr@c6d0e09 · GitHub

標準出力・標準エラー出力のどちらも有効
標準出力・標準エラー出力のどちらも有効

actions/runner のコードを追う

GitHub Actionsのworkflow commandでアノテーションするときのmessageで改行したい - 私が歌川です で、改行文字をencodeする処理の実装を追いかけたので、これを起点にコードを追う。C# を読むのに慣れてないので間違ってたら教えてください。

まとめ

  • 2020/10/13時点 (GitHub Actions runner v2.273.5) では、GitHub Actionsのwofklow commandは、標準出力と標準エラー出力のどちらに出力してもよい
    • コードを追った感じではそうなっている

ふだんの開発で小さなPRに分けるために考えていること

ふだんの開発でPRを出すときに考えていること - 私が歌川です の続編です。小さなPRを出す、という話がありましたが、どうやって小さくしているのか、というのを書きます。

どういうメソッドが欲しいか考える

  • Aという情報をBという画面に出したい
  • Aを取得するメソッドがないので、それを作る必要がある (メソッドCとする)

というときに、メソッドCを作るPRをまず先に出して、マージされたら続いてCを使って得られる情報Aを画面Bに出すPRを出す、というのをやります。 実装方針を考えて既存実装を眺めてからそういう作戦を立てることもあれば、実装しているうちにメソッドCが足りないな、と気づいて別ブランチでPRを作ってそちらを先に出す、ということもあります。

このとき、1つのPRで両方やろうとすると、メソッドCの実装のレビューと、情報Aを画面Bに出す実装のレビューが同じPRで行われることになります。メソッドCのロジックが小さいうちはなんとかなるかもしれませんが、込み入ったことをやろうとすると、レビュワーがレビューすべきこともレビュイーが修正することもどんどん多くなって大変です。

最初はシンプルなロジックだと思っていても、レビューを受けて修正を重ねていくと、だんだんしっかりしたロジックになってきたぞ、となることがあります。最初のPRがじゅうぶん小さければ、修正を重ねてもロジックは1つなのでまだ小さいPRですね、というふうにできます。

1つのPRでついでにやらない

実装が終わったあとに、既存実装部分をついでに修正したくなることがありますが、ぐっとこらえます。ここで「ついでに」と言ったのは、PRで実装したいロジックの本筋とは関係ない部分を指します。ついでにリファクタリングできるとリファクタリングができて嬉しいのですが、ロジックのレビューと、リファクタリング部分が既存ロジックを壊してないかのレビューが同時に行われることになります。

また、実装方針を考えたけど今のメソッドのインタフェースだと実現が難しいからリファクタリングしないといけない、という場合もあります。こういうときも、リファクタリングと機能実装を同時にやるのではなく、まずリファクタリングをやってPRを出して、やりたかった機能実装を次にやる、というふうに進めます。PRでは実装方針を伝えつつリファクタリングの理由を説明します。

一気に全てを良くしたくなる気持ちは分かりますが、ぐっとこらえて1つずつ解決しましょう。

分割方法が分からなかったら聞いてみる

慣れてない箇所や、馴染みのない概念に触れるときは、どうやって実装を分割するか見当がつかないことがあります。そういうときは、その箇所・概念について知ってる人に聞くのが手っ取り早いです。レビューで指摘された場合はレビュワーに聞いてみるのがよいでしょう。あるいはペアプロ・ペアレビューをするのもよさそうです。

おわりに

小さなPRに分けるために考えていることを紹介しました。こういう仕草をどこで習得したのかは忘れました。最初からこれができることを求められることはないだろうと思うので、経験を積むのがよいのではないかと思っています。機能を完成させることを目標として、完成のためのパーツを1つずつ作って組み合わせる、みたいな振る舞いかたをしている、というふうに自覚しています。

最後に、いい言葉があったので引用して記事を締めます。

小さすぎる CL を書いて失敗するほうが大きすぎる CL を書いて失敗するよりもよほど良いです。レビュアーと協力してどんなサイズの CL なら受け入れられるかを探ってみてください。

小さな CL | google-eng-practices-ja

あわせて読みたい

悪夢を見て飛び起きた。すぐに取ったメモを見返したけど何もわからなかったのでそのまま公開します。

  • 引っ越す
  • 広い部屋になってよかった
  • ある日起きると知らないおっさんがソファで寝てた
  • 寝込みを襲われそうになり通報しようとしたがスマホの電池が4%しかない
  • またあるときは野球部員が家に押し寄せてきた
  • 踏切の先が海になっているので進もうとすると足がびしょびしょになる
  • 真ん中の車両に乗ったらよかったね、死ぬほど痛いから、と言われ、慌ててバスを飛び出した

  • 階段状になってて、床に座れるような場所で、コナンの映画を見た
    • コナンが右目を損傷して灰原に手当てされる
    • 途中でアイドルが歌うシーン
    • あまりに眠すぎて、なぜか映画館なのに布団があったのでかぶって寝ていた
  • ツイートがバスってしまい大量の通知が流れてくることになった
    • 変なGIFアニメとか、甘えるな! みたいなリプライが常に飛んできて、最悪

ふだんの開発でPRを出すときに考えていること

業務の話です。OSSとかだとまた変わってくるのかもしれないし、共通することもあるかもしれません。

  • 先に作戦を練る
    • 実装する前に、方針段階でレビューしてもらえるとよい
    • 自分だけでは気づけない考慮漏れとか、こういう方針もあるよっていう提案とか、いろいろ得られるものがある
    • 先に実装完成させてから、これでは要件を満たせない・うまくいかないねってなるともったいない
  • 巨大なPRにしない
    • diffの大きさについては、プログラミング言語とか、利用するフレームワークによっても変わってくるので、一概には言えなさそう
      • +1500, -1500 だけどスナップショットの更新があったとか
      • インデント1つ下げることになったとか
    • たとえば、あらゆる機能を1つのPRで実装してたら巨大なPRになると思う
      • 1つのPRであらゆるものを実装しない、1機能ずつ実装するとか、1つの層だけ実装する、とか
    • PRが巨大だと、コミュニケーションを取りつつ延々とレビューを打ち返す、みたいな感じになってよくない
      • お互いに疲れる
      • とくにパートタイムだと、少ない労働時間でずっとやり取りを続けることになる
    • DB→モデル→サービスメソッド→コントローラー→UI みたいな感じで、少なくとも層ごとにPRを分ける
      • 分け方は諸説ありそう
    • 1つの層の中でもちょっとずつやる
      • CRUD一気に、ではなく、まずはCから、とか
      • Cの実装とテストをまとめて1つのPRとして出して、レビューしてもらう
    • PRの大きさとは何か? という問いに対して、PRが向けている関心の多さがそれにあたるのではないか、というのを思ったけど、うまく言語化できてない
  • 文脈を書く
    • こうしました!! だけ書いてあっても、なぜこうしたいのか分からない
    • 最低限、issueへのリンクは貼っておきたい
  • 動作確認の方法を書く
    • 「どういう条件で」「何を」「どうすると」「どうなれば正しい」のかが分かるように書く
    • テストが通ればオッケーならそのように書いておく
  • PRのテンプレートに従う
    • それはそうですね
    • ちゃんとしたテンプレートができていると、上の2つはだいたい満たせそう
  • テストを書く
    • テストを見たら、こういう振る舞いが想定されている、というのが分かる
    • メソッドのインタフェースとか、使われ方とか分かる
    • 機能追加やリファクタリングするときに、既存の機能が壊れてないかどうか分かる
      • 影響範囲が見積もれる
      • 軽い修正のつもりがけっこう既存の実装が壊れる、とか分かる
    • 丁寧にテストする
      • 正常系・異常系・コーナーケース
      • 不等号とか間違えがちなので、どういう入力に対してどういう出力が想定されているか、ちゃんと書く
        • よく条件を逆にするので、自信ないと思ったらテストケースを足す方に倒してる*1
      • テスト書きすぎ問題 - hitode909の日記

追記

CL の作者がコードレビューを乗り切るためのガイド | eng-practices小さなCLのページに、巨大でない・小さいPRとはどういうものかについて言語化されているのを id:shiba_yu36 さんに教えてもらいました。自分が検討違いなことを言ってないかちょっと気にかけてたけど、これを読む限りでは間違ったことは言ってなさそうでした。

リファクタリングを先にやって差分が大きくならないようにしましょう、CL*2をどうしても小さくできないように見えるときも、検討に検討を重ねましょう、それでもCLが大きくなってしまうと分かったら十分に注意しましょう、ということが述べられていて同意があります。

大学近くに最近できたラーメン屋に行った。

tabelog.com

あくた川流 野菜増し

ラーメンおいしかった。ラーメン二郎京都店をさらにマイルドにした感じ。肉を増すとボリューム感を再現できそう。よい二郎インスパイヤだったので、これからも通うことになるかもしれない。


昨日はISUCON10本戦後に飲みに行ってた。起きてちょっと作業してて、ラーメン二郎が食べたい気持ちになったけど京都店は今日は閉めてて困ったけど、あくた川流は二郎インスパイヤと聞いてたので向かうことにした。

さて、食券機に向かって、麺の量はどうしようか、野菜は増すか? など考えながら財布を開いたら、600円ほどしか入ってなかった。マネーフォワードを見ると財布の中身は20円ということになってて、いずれにせよこれではラーメンを食べることができない。

思い返すと、二次会までやって、酔っ払いの面倒見などもありわちゃわちゃしていたので、現金管理が崩壊している。店員さんに、すいませんお金が足りなかったのでおろしてから来ます、と伝えて近くのローソンに走った。

ラーメンを食べるのに十分なお金は持っているのに、現金を持ち歩いていないだけでこういう体験をすることになるのは、惨めな気持ちになる。手持ちのお金がどれくらいかを常に気にしながら生活するのは困難を極める。これが食べる前だったからよかったけど、食べ終わってから、どれ、支払いをしましょうかね、というタイミングになって、財布の中身がないことに気づくとさらに深刻な事態になっていたと考えられる。

できるだけ現金を持ち歩きたくなくて、というかちょっと前までは持ち歩くほどの現金が手元になかったので、できるだけクレジットカードで暮らしていたのだけれど、現金がちょっとはないと詰んでしまうこともありえる。タクシーを拾うときに、クレジットカードやPayPayでの支払いに対応しているかどうかを確認してから呼び止めるような暇はない。まとまったお金をおろして一ヶ月過ごす、というのをやっているけど、現金支払いの見込みと大きくずれるとこういう事故が起こりうる。

個人間送金も難しく、Kyashの送金が気軽に行えなくなったので、がんばって割り勘精算を行う生活に逆戻りしている。とにかく手軽に送金したい。