Skip to content

Conversation

@zztkm
Copy link
Contributor

@zztkm zztkm commented Feb 12, 2025

SwiftFormat から swift-format へ移行する対応で、swift format lint によるスタイルチェックを有効化し、すべての警告を修正しました。

  • [CHANGE] フォーマッターを swift-format に移行する
    • SwiftFormat のための設定ファイルである .swiftformat と .swift-version を削除
    • フォーマット設定はデフォルトを採用したため、.swift-format は利用しない
    • swift-format のデフォルト設定で、format lint を行った結果、警告が出た部分はすべて修正
    • JSON デコード処理に使う JSON のキー名を指定するための enum の定義については、AlwaysUseLowerCamelCase ルールを無効化するためのコメントを追加
      • シグナリングメッセージのキー名にスネークケースが採用されている項目があるため、この対応を行った
  • [UPDATE] GitHub Actions で Lint を行うコマンドを Makefile に変更
    • 今まで lint-format.sh で一括実行したところを Makefile に移行したので、GitHub Actions でも Makefile を利用するように変更
    • lint-format.sh は利用しなくなったので削除

This pull request includes several changes to improve the codebase by updating documentation comments and modifying the build and lint process. The most important changes include updating the GitHub Actions workflow to use a Makefile for linting, transitioning documentation comments from block comments to line comments, and simplifying switch case syntax.

Build and Lint Process Updates:

  • .github/workflows/build.yml: Added a new step for format linting and updated the linting step to use make lint instead of ./lint-format.sh.
  • CHANGES.md: Documented the transition to using a Makefile for linting in GitHub Actions, including the removal of lint-format.sh. [1] [2]

Documentation Comment Updates:

Code Simplification:

@zztkm zztkm self-assigned this Feb 12, 2025
- name: Lint
run: |
./lint-format.sh
make lint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@zztkm zztkm changed the title [WIP] lint-format.sh から Makefile に移行する [WIP] CI に Format Lint を追加する Feb 13, 2025
Comment on lines 38 to 39
- [UPDATE] GitHub Actions で format check をするのをやめる
- @zztkm
Copy link
Contributor Author

@zztkm zztkm Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 format check は一時的にはやめたけど今回戻したのでこの CHANGES は混乱を生みそうな気がした。
結果的に SwiftFormat から swift-format に format lint も移行しただけなので消しておいて良いかもしれない。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format lint と lint を GitHub Actions で実行する箇所とローカルでの lint 実行は Makefile に譲ったので、lint-format.sh は不要になった。今までありがとう。

// 採用された WebSocket 以外を切断してから webSocketChannelCandidates をクリアする
weakSelf.webSocketChannelCandidates.removeAll { $0 == webSocketChannel }
weakSelf.webSocketChannelCandidates.forEach {
for candidate in weakSelf.webSocketChannelCandidates {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forEach ではなく、for in が推奨されていたので対応(他の forEach を使っている箇所も同様。


switch signaling {
case var .offer(message):
case .offer(var message):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let と同様、var も () 内に定義する(警告文では let と表記されるけど...)。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

フォーマッターに従う方針で良いと思います。

Comment on lines +649 to +650
// swift-format-ignore: AlwaysUseLowerCamelCase
// JSON のキー名に合わせるためにスネークケースを使用しているので、ignore する
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGES にも記載したが、JSON デコード処理に使う JSON のキー名を指定するための enum の定義については、AlwaysUseLowerCamelCase ルールを無効化するためのコメントを追加。

Signaling.swift 内に数か所同じコメントがある。

@zztkm zztkm requested a review from torikizi February 13, 2025 06:16
@zztkm zztkm changed the title [WIP] CI に Format Lint を追加する CI に Format Lint を追加する & Format Lint の警告に対応 Feb 13, 2025
Copy link
Contributor

@torikizi torikizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM です。修正ありがとうございました。

@zztkm zztkm merged commit 9edc082 into develop Feb 13, 2025
4 checks passed
@zztkm zztkm deleted the feature/add-ci-fmt-lint branch February 13, 2025 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants