Skip to content
This repository was archived by the owner on Jun 28, 2019. It is now read-only.

Docの追加、内部処理改善 #7

Merged
merged 6 commits into from
Sep 14, 2018
Merged

Docの追加、内部処理改善 #7

merged 6 commits into from
Sep 14, 2018

Conversation

koji-1009
Copy link
Contributor

No description provided.

@koji-1009 koji-1009 added the enhancement New feature or request label Sep 13, 2018
@koji-1009 koji-1009 self-assigned this Sep 13, 2018
Copy link

@nacatl nacatl left a comment

Choose a reason for hiding this comment

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

コードの方はLGTMかと思います。
英文だけ自分ならこう書くかなくらいにコメント書いときました

data?.also {
CertificationStore.create(context).update(it)
if (data == null) {
Log.e("StudyplusSDK", "The data is null. Please check your code. If the received data is already null, please contact Studyplus Dev team.")
Copy link

Choose a reason for hiding this comment

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

badge
個人的に already が違和感あるんですが、なんかうまく言い表せない…

If the data that received from Studyplus app is null, 

の方が、 何がnullだったら をより正確に表せるような気はします

Copy link
Contributor Author

Choose a reason for hiding this comment

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

修正します!

fun postRecord(context: Context, studyRecord: StudyRecord, listener: OnPostRecordListener?) {
if (!isAuthenticated(context)) {
throw IllegalStateException("Please check your application's authentication before this method call.")
Copy link

Choose a reason for hiding this comment

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

badge
先に認証をチェックしてくれ と言うより 先に認証してくれ の方が自然な気がします
Please authenticate (your application) before this method call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SDKの利用デベロッパーに「チェックして認証されていなければ叩かない(先に認証してから叩く)」ことを通知したいため、こちらはこのメッセージでいきたいです。

Copy link
Contributor

@nabesuke0 nabesuke0 left a comment

Choose a reason for hiding this comment

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

LGTM

@koji-1009 koji-1009 merged commit 9fb4a44 into master Sep 14, 2018
@koji-1009 koji-1009 deleted the develop branch September 14, 2018 06:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants