-
Notifications
You must be signed in to change notification settings - Fork 200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
整理: CLI 引数を CLIArgs
クラスで型付け
#1401
Conversation
#1404 の Go 判断に基づき、draft を解除しました。レビューよろしくお願いします。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ほぼLGTMです!
タイトルの
整理: CLI 引数をクラス化して型付け
ですが、引数をクラス化しているわけではないので誤解が生まれるかもです。
ちなみにdataclassは値の検証を行わないので、今の段階だとvalidationされないはずです。
validationまで実装しちゃうのはどうでしょう?
👍️
#1404 にて |
あったしかにです、そちらを失念してました!! 気になってたのはargparseから構造体にcastする際にバリデーションがない点でした! サクッとできそうならお願いしたみです・・・! |
👍️ @Hiroshiba |
@Hiroshiba |
@Hiroshiba |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
内容
CLI 引数をクラス化して一括型付けするリファクタリングを提案します。
関連 Issue
part of #1404