oss-gate / workshop

OSSの開発に未参加または参加したことはあるけどまだ自信がない人を後押しするワークショップ用のリポジトリー
125 stars 547 forks source link

OSS Gate Workshop: PHP Lovers: 2024-02-12: azuki-penguin: Lighthouse: Work log #1785

Closed azuki-penguin closed 9 months ago

azuki-penguin commented 9 months ago

This is a work log of a "OSS Gate workshop". "OSS Gate workshop" is an activity to increase OSS developers. Here's been discussed in Japanese. Thanks.

作業ログ作成時の説明

以下のテンプレートを埋めてタイトルに設定します。埋め方例はスクロールすると見えてきます。

OSS Gate Workshop: ${LOCATION}: ${YEAR}-${MONTH}-${DAY}: ${ACCOUNT_NAME}: ${OSS_NAME}: Work log

タイトル例↓:

OSS Gate Workshop: Tokyo: 2017-01-16: kou: Rabbit: Work log

OSS Gateワークショップ関連情報

azuki-penguin commented 9 months ago

Lighthouse の repo の LICENSE を確認した。

https://github.com/nuwave/lighthouse/blob/master/LICENSE

MIT ライセンスの OSS.

azuki-penguin commented 9 months ago

Contribution の方法を確認。

https://github.com/nuwave/lighthouse/blob/master/CONTRIBUTING.md

Docker Compose と Make を利用している。 セットアップは make setup, 変更した際にテストするには make を実行すれば良さそう。

composer や PHP でやる方法もあるが、環境の準備が必要なので、 Docker Compose + Make の方が楽そう。

azuki-penguin commented 9 months ago

テストデータの設定用のコード例が以下に書かれている。 https://github.com/nuwave/lighthouse/blob/master/CONTRIBUTING.md#test-data-setup

azuki-penguin commented 9 months ago

要確認: protobuf ページタイトルには Protocol Buffers と書かれているが、知らない概念

azuki-penguin commented 9 months ago

Protocol Buffer: 言語やプラットフォームに依存しない、データ構造を selialize するための仕組み

azuki-penguin commented 9 months ago

federate tracing のために protobuf を使っているらしい。 データ構造らしきものが以下に書かれていた。 https://github.com/nuwave/lighthouse/blob/master/src/Tracing/FederatedTracing/reports.proto

azuki-penguin commented 9 months ago

Lighthouse のユーザーから使われるクラスやメソッドには、 @api の PHPDoc タグをつけること。 https://github.com/nuwave/lighthouse/blob/master/CONTRIBUTING.md#internal

azuki-penguin commented 9 months ago

CHANGELOG の書き方の方針が書かれている。 CHANGELOG に追記する際には、 Unreleased セクションに追記すること。 https://github.com/nuwave/lighthouse/blob/master/CONTRIBUTING.md#changelog

実際の CHANGELOG は以下 https://github.com/nuwave/lighthouse/blob/master/CHANGELOG.md

azuki-penguin commented 9 months ago

拡張性のために、 private より protected を推奨 テストでは final を使う

Laravel と Lumen の両方で compatible にするために、 Facade は使わない。 代わりに DI を利用する。

また、ヘルパー関数の代わりに Illuminate class を利用する方が良い。 ただし、 response() は例外で、 response() を利用すること。 (Lumen でも使える、ResponseFactory は Lumen で使えない)

https://github.com/nuwave/lighthouse/blob/master/CONTRIBUTING.md#code-guidelines

azuki-penguin commented 9 months ago

テストをより厳格化するために、型情報は PHPDoc の記法に準拠する。 Collection 系は Java などの generic の書き方で各要素の型を指定できる。

インスタンス自体を返すメソッドでは、返却型に self と入れれば良いらしい。

class Foo
{
    /**
     * Some attribute.
     */
    protected string $bar;

    /**
     * Use $this for fluent setters when we expect the exact same object back.
     *
     * @return $this
     */
    public function setBar(string $bar): self
    {
        $this->bar = $bar;

        return $this;
    }
}
azuki-penguin commented 9 months ago

コードフォーマットは make fix で整形可能

azuki-penguin commented 9 months ago

PHPDoc に型情報を書く場合は、 full namespace で書くこと。 (コード内に use 文があっても、常に full namespace で記載)

azuki-penguin commented 9 months ago

ベンチマーク測定は make bench で実行できる。

azuki-penguin commented 9 months ago

CONTRIBUTING.md を読了。 ドキュメントがしっかりしているので、迷ったら参考にすること。

azuki-penguin commented 9 months ago

repo を fork. https://github.com/azuki-penguin/lighthouse

azuki-penguin commented 9 months ago

make setup を実行したらコケた。

docker-compose up --detach
[+] Running 21/21
 ✔ redis 8 layers [⣿⣿⣿⣿⣿⣿⣿⣿]      0B/0B      Pulled                        9.2s 
   ✔ c57ee5000d61 Already exists                                           0.0s 
   ✔ 01e3a03de6cc Pull complete                                            1.4s 
   ✔ 1fb20eb53a94 Pull complete                                            0.6s 
   ✔ 7e3dfb4dc4b5 Pull complete                                            2.2s 
   ✔ 7edd0a34dadd Pull complete                                            3.9s 
   ✔ 0a0eb0ff91b9 Pull complete                                            2.3s 
   ✔ 4f4fb700ef54 Pull complete                                            3.7s 
   ✔ b56c0201b4fa Pull complete                                            3.8s 
 ✔ mysql 11 layers [⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿]      0B/0B      Pulled                   54.2s 
   ✔ 20e4dcae4c69 Pull complete                                           24.0s 
   ✔ 1c56c3d4ce74 Pull complete                                            4.2s 
   ✔ e9f03a1c24ce Pull complete                                            5.3s 
   ✔ 68c3898c2015 Pull complete                                            6.8s 
   ✔ 6b95a940e7b6 Pull complete                                            6.8s 
   ✔ 90986bb8de6e Pull complete                                            7.7s 
   ✔ ae71319cb779 Pull complete                                           23.7s 
   ✔ ffc89e9dfd88 Pull complete                                            8.8s 
   ✔ 43d05e938198 Pull complete                                           19.8s 
   ✔ 064b2d298fba Pull complete                                           20.8s 
   ✔ df9a4d85569b Pull complete                                           21.5s 
[+] Running 5/5
 ✔ Network lighthouse_default    Created                                   0.1s 
 ✔ Container lighthouse-node-1   Started                                   1.2s 
 ✔ Container lighthouse-mysql-1  Started                                   1.2s 
 ✔ Container lighthouse-redis-1  Started                                   1.2s 
 ✔ Container lighthouse-php-1    Created                                   0.1s 
Error response from daemon: unable to find user so16: no matching entries in passwd file
make: *** [up] Error 1
azuki-penguin commented 9 months ago

make setup 実行時に、 idコマンドのエラーが表示されていた。 ただ、そのまま実行は続いた。

$ make setup 
id: illegal option -- -
usage: id [user]
       id -A
       id -F [user]
       id -G [-n] [user]

       id -P [user]
       id -g [-nr] [user]
       id -p [user]
       id -u [-nr] [user]
id: illegal option -- -
usage: id [user]
       id -A
       id -F [user]
       id -G [-n] [user]

       id -P [user]
       id -g [-nr] [user]
       id -p [user]
       id -u [-nr] [user]
docker-compose build --pull --build-arg USER_ID= --build-arg GROUP_ID=
[+] Building 229.0s (17/17) FINISHED                       docker:desktop-linux
azuki-penguin commented 9 months ago

make setup を実行すると、 USER_IDGROUP_ID の環境変数で id コマンドが使われている。 macOS の id コマンドでは、 --user--group オプションは存在しないため、エラーが発生する。 (Linux の場合は通りそう?)

 11 .PHONY: setup
 12 setup: build docs/node_modules vendor ## Setup the local environment
 13 
 14 .PHONY: build
 15 build: ## Build the local Docker containers
 16         docker-compose build --pull --build-arg USER_ID=$(shell id --user) --build-arg GROUP_ID=$(shell id --group)
azuki-penguin commented 9 months ago

修正は Linux, Mac の両方の環境で動くことを確認しないといけないので、とりあえず bug 報告を投げることにする。

azuki-penguin commented 9 months ago

issue に mac id, id docker で検索をかけてみたが、上記のバグ報告はなさそうだった。

azuki-penguin commented 9 months ago

issue テンプレートがあったので、下書き。


Describe the bug

The command make setup failed on macOS. When I ran the command, the following error message appeared.

Click to expand to show the error message. ``` $ make setup id: illegal option -- - usage: id [user] id -A id -F [user] id -G [-n] [user] id -P [user] id -g [-nr] [user] id -p [user] id -u [-nr] [user] id: illegal option -- - usage: id [user] id -A id -F [user] id -G [-n] [user] id -P [user] id -g [-nr] [user] id -p [user] id -u [-nr] [user] docker-compose build --pull --build-arg USER_ID= --build-arg GROUP_ID= ```

id command on macOS doesn't have --user and --group options, so the environment variables USER_ID and GROUP_ID was set empty.

docker-compose build was executed, but docker-compose up --detach failed because of the following error.

Click to expand to show the message that said the setup failed. ``` docker-compose up --detach [+] Running 21/21 ✔ redis 8 layers [⣿⣿⣿⣿⣿⣿⣿⣿] 0B/0B Pulled 9.2s ✔ c57ee5000d61 Already exists 0.0s ✔ 01e3a03de6cc Pull complete 1.4s ✔ 1fb20eb53a94 Pull complete 0.6s ✔ 7e3dfb4dc4b5 Pull complete 2.2s ✔ 7edd0a34dadd Pull complete 3.9s ✔ 0a0eb0ff91b9 Pull complete 2.3s ✔ 4f4fb700ef54 Pull complete 3.7s ✔ b56c0201b4fa Pull complete 3.8s ✔ mysql 11 layers [⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿] 0B/0B Pulled 54.2s ✔ 20e4dcae4c69 Pull complete 24.0s ✔ 1c56c3d4ce74 Pull complete 4.2s ✔ e9f03a1c24ce Pull complete 5.3s ✔ 68c3898c2015 Pull complete 6.8s ✔ 6b95a940e7b6 Pull complete 6.8s ✔ 90986bb8de6e Pull complete 7.7s ✔ ae71319cb779 Pull complete 23.7s ✔ ffc89e9dfd88 Pull complete 8.8s ✔ 43d05e938198 Pull complete 19.8s ✔ 064b2d298fba Pull complete 20.8s ✔ df9a4d85569b Pull complete 21.5s [+] Running 5/5 ✔ Network lighthouse_default Created 0.1s ✔ Container lighthouse-node-1 Started 1.2s ✔ Container lighthouse-mysql-1 Started 1.2s ✔ Container lighthouse-redis-1 Started 1.2s ✔ Container lighthouse-php-1 Created 0.1s Error response from daemon: unable to find user so16: no matching entries in passwd file make: *** [up] Error 1 ```

I checked this error on following environment.

Expected behavior/Solution

id command should finish successfully and the error message should NOT appear when make setup executed. And then, docker-compose up --detach should finish successfully.

Steps to reproduce

  1. Execute make setup on macOS

Lighthouse Version

master (a97436fbf375f8ba90d865b236ed05617083c212)

azuki-penguin commented 9 months ago

issue を投げた。 https://github.com/nuwave/lighthouse/issues/2503

azuki-penguin commented 9 months ago

macOS の id コマンドの Man Page は以下の通り。 https://ss64.com/mac/id.html

Linux の id コマンドの Man Page は以下の通り。 https://man7.org/linux/man-pages/man1/id.1.html

--user の代わりに -u, --group の代わりに -g を使えば、 macOS でも Linux でも動作すると予想される。

azuki-penguin commented 9 months ago

上記の変更で macOS で make setup が通ったので、 PR を投げる。 PR の説明の下書きを作成。


Resolves #2503

If updating CHANGELOG.md is necessary, could you tell me that? I'll update it, too.

Changes

Changed id command options in Makefile to finish successfully both macOS and Linux.

Notes

I checked that make setup finished successfully on macOS, but I've not checked whether make setup also finish successfully on Linux. The environment I checked is below.

azuki-penguin commented 9 months ago

PR を作成した。 https://github.com/nuwave/lighthouse/pull/2504

github-actions[bot] commented 9 months ago

おつかれさまでした!

ワークショップの終了にともないissueを閉じますが、このまま作業メモとして使っても構いません :ok_hand:

ワークショップの感想を集めています!

ブログなどに書かれた際は、このページへリンクの追加をお願いします :pray:

またの参加をお待ちしています!