-
Notifications
You must be signed in to change notification settings - Fork 2
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
指定した GitHub Discussion ID にコメントを追加する reusable workflow #60
base: main
Are you sure you want to change the base?
Conversation
@masutaka なにとぞ 🙏🏻 🙇🏻 |
あと、直接関係はないのですが、このワークフローと、https://github.com/route06/actions/blob/main/.github/workflows/create_discussion.yml とを組み合わせて使う場合、 https://github.com/route06/actions/blob/main/.github/workflows/create_discussion.yml のほうにdiscussion idをoutputとしてもたせる実装を入れる想定で良さそうでしょうか? 💭 |
@TomckySan 今は reusable workflow を増やすよりも、まだ未リリースの下記 reusable workflow の完成度を高めたほうが良いと思います。
今は feature ブランチが main ブランチに未マージの状態です。個人的には Release PR #51 をマージして v2.2.0 をリリースしてから、次の機能を実装していきたいです。今は別件で他にリリースしたいものがあってもリリースできない状態なので、これを収束させてから、本 PR の議論を進めていきたいです。 |
👀 |
ready for reviewにしますた |
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.
方向性としては良さそうに思いました。
コメントしましたが、comment_seed_path の中身が難しそうですね💨
// seedのイメージ | ||
// {"template_file_path": "xxx/yyy/zzz.md", "replace_values": {"title": "たいとる", "assign": "たなか", "foo": "こめんと"}} | ||
// seedファイルに指定されているファイルをテンプレートとしてコメントを作成 | ||
const template = fs.readFileSync(seed['template_file_path'], "utf8") |
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.
must:
ここのイメージがうまく掴めませんでした🙏
実際にこの workflow を使うサンプルコードがあると分かりやすいと思います。
あと、この comment_seed_path の中身は、ユースケースごとに違いそうなので、どう需要に応えるか/応えないかが今回の肝かと思いました。
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.
そうなんですよね、ここ悩みどころでした。少し考えてみます 💭
サンプルは用意します!
env: | ||
TZ: "Asia/Tokyo" |
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.
may: 要らないかも?
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.
imo:
ファイル名の案:
- add_gh_discussion_comment.yml
- 他 workflow と同じように
gh
は付けたほうが良い
- 他 workflow と同じように
- add_comment_to_gh_discussion.yml
- もう少し具体的な名前にする
- append_comment_to_gh_discussion.yml
- append で後ろに追加するニュアンスにする?
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.
nit: 全体的に YAML のインデントは 2 でお願いしますw
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.
まじごめんなさいwww 🙏🏻 🙇🏻
はい、良いと思います。 |
最新の main ブランチの commit を取り込んで、この PR で進めて良いと思います! |
ファイルが無い場合や、ファイルの中身が期待通りでなかった場合など、考慮点は追って実装予定。
まずこのPRでみていただきたいのは、方針・設計が問題ないか、という点。
その点についてコメントいただき、問題なければ細かいところを詰め、別ブランチで再度PRを上げたい。