-
Notifications
You must be signed in to change notification settings - Fork 12
Feature/23 akatonbo #23
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
Conversation
sago35
left a comment
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.
PR ありがとうございます。
とてもよいです。
2 点お願いと、 1 点相談 (or 質問) です。
■お願い1
README.md の末尾の作例のところに足してほしいです。 ./21_midi2 のあとが良いかな。
■お願い2
Makefile の smoketest に追加お願いします。
22_buzzer のあとが良いです。
■相談 (or 質問)
go.mod は、 ./23_akatonbo/go.mod ではなく、 ./go.mod を使ってほしいです。
大きな理由がある場合は教えてください。
今後、 go.mod で依存を Update していきますが、原則として go.mod を udpate した時点の Version ですべての例が動く、という状態をキープしたいと思っています。
23_akatonbo/go.mod
Outdated
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.
go.mod と go.sum については、大きな理由がない限りは一つ上の go.mod を共通で使うようにしてください。
一つ上の go.mod は drivers とかが古いですが、適宜 update する PR も歓迎です。
今回の場合は update しなくてもそのまま再生できます。
今のままだと、 tinygo flash ./23_akatonbo という実行ができず、cd ./23_akatonbo && tinygo flash . のような形となります。
これは他の例と違う形になるので初学者が混乱する側かな、と。
なお、一つ上の go.mod を使った場合は、上記のどちらのコマンドも実行できます。
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.
修正しました
|
修正しました! |
sago35
left a comment
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.
.gitignore はこの PR とは関係がないので不要ではある。
が、特に問題もないので merge します。
LGTM && Thank you.
This PR adds a new example project implementing a Japanese music box that plays "Akatonbo" (Red Dragonfly), a beloved traditional Japanese song.