Skip to content
This repository was archived by the owner on Sep 27, 2023. It is now read-only.

Features/company#99

Merged
t2h5 merged 8 commits intodevelopmentfrom
features/company
Nov 11, 2022
Merged

Features/company#99
t2h5 merged 8 commits intodevelopmentfrom
features/company

Conversation

@t2h5
Copy link
Copy Markdown
Contributor

@t2h5 t2h5 commented Nov 7, 2022

  • companyのデータを取得できるようにする

image

@t2h5 t2h5 self-assigned this Nov 7, 2022
@t2h5 t2h5 force-pushed the features/company branch from 0f7c6ff to 14c5d90 Compare November 7, 2022 10:29
Copy link
Copy Markdown
Contributor Author

@t2h5 t2h5 left a comment

Choose a reason for hiding this comment

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

第2引数に COMPANY を指定して企業情報を取得できるようにしました

Comment thread src/main.ts Outdated
@t2h5 t2h5 requested a review from akiomik November 11, 2022 02:27
@t2h5 t2h5 marked this pull request as ready for review November 11, 2022 02:27
Comment thread src/__mocks__/api/v3/client.ts
jest.clearAllMocks()
})

test('get', () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

すみません、 getput のテストも一応書いてもらえると!
quarterやdailyにテストが存在してないのは追加し忘れか手抜きだと思います… 🙇

大元は getXXXputXXX しかなくて、 getput は後から追加したものです。
getXXXputXXX は移行のためにpublicなメソッドになってますが、おそらく使ってるところはもうないため、順次privateに切り替えていって、 getput だけテストする世界線にした方がわかりやすいかなと。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

修正してみました
020c0f8

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💯

Comment thread src/main.ts Outdated
Co-authored-by: Akiomi Kamakura <akiomik@gmail.com>
Comment thread src/custom-functions/v3/bcode.ts Outdated
Comment thread src/custom-functions/v3/bcode.ts Outdated
Copy link
Copy Markdown
Contributor

@akiomik akiomik left a comment

Choose a reason for hiding this comment

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

コメントしました。メインの処理は良さそうでした 👍

@t2h5 t2h5 requested a review from akiomik November 11, 2022 03:24
Comment thread src/setting.ts
static readonly defaultOndemandApiEnabled = false
static readonly defaultOndemandApiCallMode = Setting.ondemandApiCallModes.DEFAULT

static readonly companyIntent = 'COMPANY'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
Contributor

@akiomik akiomik left a comment

Choose a reason for hiding this comment

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

バッチリでした 👍

@shoe116
Copy link
Copy Markdown
Contributor

shoe116 commented Nov 11, 2022

第2引数をなんて呼ぶか だけ決めてしまいたいな
Excel のほうでも問題になってた( Excel の方は一旦 period のままにしてある

Copy link
Copy Markdown
Contributor

@shoe116 shoe116 left a comment

Choose a reason for hiding this comment

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

intent で合意取れたので intent で!

@t2h5 t2h5 merged commit 09e39f8 into development Nov 11, 2022
@t2h5 t2h5 deleted the features/company branch November 11, 2022 06:02
@t2h5 t2h5 mentioned this pull request Nov 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants