Skip to content
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

another implementation of "fetch-messages" #116

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

koron
Copy link
Member

@koron koron commented May 11, 2020

fetch-messages の別実装を書いてみました。

API自身とをイテレートするところと取得結果をファイルに書き出すところをそれぞれ抽象化して
fetch-messages からどう使うかが見どころです。

ConversationsHistory の中身は書いてないですが
そこは見せたい場所ではないので省略してます。


パッケージ解説

  • internal/slackadapter/ - Slack API を実際に叩くところ。イテレート/ページネーションのためのユーティリティ関数を含む
  • internal/jsonwriter/ - JSONでの永続化層。実装はクッソ適当だがI/FがWriteとCloseとして切れてるあたりがみそ。あとで差し替えるときも楽。
  • subcmd/fetchmessages/ - fetch-messages サブコマンドの実装位置。サブコマンド間のnamespaceが変に混ざらないように分けたほうが吉

そのほか

contex.Context は今はこういものと覚えてほしいです。
タイムアウトとかキャンセルができたほうが良い
時間のかかることをするときに使います。
たとえばネットワークアクセスとか。

@koron koron requested review from thinca, tennashi and kuuote May 11, 2020 01:06
@koron koron force-pushed the slackapi-adapter-draft branch 5 times, most recently from 64024c9 to ef61adb Compare May 11, 2020 01:27
@koron
Copy link
Member Author

koron commented May 16, 2020

#123 がマージされたら rebase して実装できる。

@koron koron force-pushed the slackapi-adapter-draft branch 2 times, most recently from fb10089 to 98d82b3 Compare May 17, 2020 02:39
@koron koron force-pushed the slackapi-adapter-draft branch from 98d82b3 to ef75709 Compare May 17, 2020 02:42
@kyoh86 kyoh86 mentioned this pull request Jul 6, 2020
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.

1 participant