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.

Sign up for GitHub

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

Jump to bottom

SDKを一部更新しました / enhancement for SDK #13

Merged
merged 10 commits into from
Apr 8, 2020
Merged

SDKを一部更新しました / enhancement for SDK #13

merged 10 commits into from
Apr 8, 2020

Conversation

uzuna
Copy link
Contributor

@uzuna uzuna commented Dec 6, 2019 •
edited by moznion
Loading

内容

  • go modules対応しました
  • APIに対して不足しているメソッドをいくつか実装しました
  • CreateEventHandlerOptions を組み立てやすくする関数を追加しました
  • GitHub actionsでテストが動くようにしました

背景

実際使用するにあたり必要なREST APIをたたく実装ない、あるいは使うのにドキュメントを精読しなければリクエストボディが組み立てられず使いづらい点があったため、使いやすくなるように修正を加えました。

修正にあたり正しく動作していること、APIとの実装に差がないことが明示できるようにGitHub Actionsで外部APIをたたくTestが動くように設定を追加しました。

Edited by maintainers

This pull request contains the following changes:

  • change to use go mod
  • setup the CI on GitHub Actions
  • Implement the feature for some lacked APIs
    • listSessionEvents
    • suspendSubscribers
    • setSubscriberToStandby
  • Add a helper method to construct CreateEventHandlerOptions structure

@moznion moznion requested review from moznion and bearmini March 29, 2020 09:57
moznion
moznion suggested changes Apr 1, 2020
View reviewed changes
Copy link
Contributor

@moznion moznion left a comment

Choose a reason for hiding this comment

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

反応が遅くなり大変申し訳ございません．pull-requestをお送りいただきありがとうございます！

レビューコメントでは網羅的に示していないのですが，golintが出力している警告についても全般的にご対応いただければ幸いです。
どうぞよろしくお願いいたします。

@bearmini なにか他にレビュー事項等あればfollow upいただければと思います．

.github/workflows/go.yml Outdated Show resolved Hide resolved
event.go Outdated Show resolved Hide resolved
sdk_test.go Outdated Show resolved Hide resolved
event.go Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
@moznion moznion added enhancement waiting for author waiting for a reaction from the author labels Apr 1, 2020
moznion added a commit that referenced this pull request Apr 3, 2020
@moznion
Make Metadata tests to be controllable whether "run" or "skip" by an …
f1790b0 
…environment variable

Rel: #13
#13 (comment)
@moznion moznion mentioned this pull request Apr 3, 2020
Merged
@uzuna
add golint and fix lint errors
1019d20 
* fix make file
* fix setup of github action
* fix comment and lint error
uzuna
uzuna commented Apr 5, 2020
View reviewed changes
.github/workflows/go.yml
run: |
go get -v -t -d ./...

- name: 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.

go installしてもgolintにpathが通らなかったためgithub actionsのissueを参考に追加しました。
github actionでの実現方法は他にいくつかありそうでしたが本PRの目的からずれるため依存が一番少ないと思われる方法をとりました。

Copy link
Contributor

Choose a reason for hiding this comment

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

ありがとうございます，こちらで良いかと存じます

@moznion moznion added waiting for reviewer waiting for a reaction from the reviewer and removed waiting for author waiting for a reaction from the author labels Apr 8, 2020
moznion
moznion approved these changes Apr 8, 2020
View reviewed changes
Copy link
Contributor

@moznion moznion left a comment

Choose a reason for hiding this comment

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

ご対応くださいまして誠にありがとうございました！
残っているgolintのエラーに関しましては既存のコードの問題かと存じますので，別のp-rにて対応したいと思います．

bearmini
bearmini approved these changes Apr 8, 2020
View reviewed changes
Copy link
Contributor

@bearmini bearmini left a comment

Choose a reason for hiding this comment

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

ご対応ありがとうございました！

@moznion moznion merged commit 2214d86 into soracom:master Apr 8, 2020
@moznion moznion changed the title SDKを一部更新しました SDKを一部更新しました / enhancement for SDK Apr 8, 2020
@moznion moznion removed the waiting for reviewer waiting for a reaction from the reviewer label Apr 8, 2020
@moznion moznion mentioned this pull request Apr 8, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moznion moznion moznion approved these changes

@bearmini bearmini bearmini approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@uzuna @bearmini @moznion