Skip to content

言語処理100本ノックのうち、00〜09までの回答です。 #10

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
Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

言語処理100本ノックのうち、00〜09までの回答です。 #10

wants to merge 14 commits into from

Conversation

tosite
Copy link
Collaborator

@tosite tosite commented Oct 16, 2018 •
edited
Loading

目標とするもの

Rubyの基礎を学ぶとともに、基本的な構文などが間違っていないかをチェックしていただきます。
併せてGithubの使い方にも慣れていきます。

利用したサイト

言語処理100本ノック
今回は00〜09までを解きました。

レビューしていただきたい方

マイルストーン

今週末（10/19）までにはマージしたいです。

レビュー観点

Rubyの構文・記述などに不備がないかチェックをお願いいたします。

マージ条件

一通りのレビューを受け、対応したらマージします。

@udzura
Copy link
Contributor

udzura commented Oct 16, 2018

やっとりますな〜

udzura
udzura reviewed Oct 16, 2018
View reviewed changes
ranka/02-answer.rb Outdated
arr2 = buf2.split('')

# ループ回数の上限を取得
cnt = (arr1.length >= arr2.length) ? arr1.length : arr2.length
Copy link
Contributor

Choose a reason for hiding this comment

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

これは丁寧な書き方で良いのですが、Rubyだと、もっと直感的な書き方があったりします

@june29
Copy link

june29 commented Oct 16, 2018

ファイルの末尾には改行があった方がいいので、すべてのファイルでそうしておくとよいでしょう。そうしないと、これから幾度となくレビューで言及されると思います。エディタの設定で「ふつうにやっていたらそうなる」としちゃうのがオススメです！

「ファイル 末尾 改行」あたりでウェブ検索するといろいろと知見が得られると思います！

june29
june29 reviewed Oct 16, 2018
View reviewed changes
ranka/01-answer.rb Outdated
@@ -0,0 +1,11 @@
buf = 'パタトクカシーー'
# 文字列を配列に展開
arr = buf.split('')
Copy link

Choose a reason for hiding this comment

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

最近の Ruby だとそれ用のメソッドがあるはず 💡

june29
june29 reviewed Oct 16, 2018
View reviewed changes
@komagata
Copy link

ファイル末尾の改行

どうぞ！

なぜ最終行に改行が必要なのか - komagataのブログ

june29
june29 reviewed Oct 16, 2018
View reviewed changes
ranka/03-answer.rb Outdated
@@ -0,0 +1,9 @@
buf = 'Now I need a drink, alcoholic of course, after the heavy lectures involving quantum mechanics.'
ans = ''
Copy link

Choose a reason for hiding this comment

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

= の位置が不思議だなあ、と感じました 👀

@june29
Copy link

june29 commented Oct 16, 2018

Pull Request のタイトルと中身がマッチしていないように見えて「やりたいことをやれているかしら？」となりました。

ここから、どのように進められると @tosite0345 さんはうれしいですか？

@tosite
Copy link
Collaborator Author

tosite commented Oct 16, 2018 •
edited
Loading

@june29 さん
レビューありがとうございます。
個々の対応は明日させていただきます。

今後の進め方についての個人的な希望としましては…。

  • コードを書く上でのアドバイス（改行コードなど）をいただけると嬉しいです
  • おすすめの開発環境（プラグインなど）を知りたいです
  • 複数人開発の方法やタスク粒度の分割（ Pull Request の件など）を学べると嬉しいです

などです。
コーディング部分は明日以降のブートキャンプである程度はカバーできるのかな？と思っていますが、上記のような社内で取り組まれていることや推奨されることなども学べると嬉しいです。

@june29
Copy link

june29 commented Oct 17, 2018

お返事ありがとうございます！ぼくはなんにも急いでいないので @tosite0345 さんのペースで進行してください。

言葉が足りなくて申し訳なかったのですが、ぼくが #10 (comment) で聞きたかったことは、この Pull Request のタイトルと中身の乖離についてのことでした 🙏

  • タイトルを尊重して、もう 00 は済んでいるのでマージする
  • 中身に合わせて、タイトルの方を変える

の二択になるかな〜と思っています。 どっちを採るかは、Pull Request の Author である @tosite0345 さんがどうしたいか次第だろうと思い、質問しました 💨

@tosite
Copy link
Collaborator Author

tosite commented Oct 17, 2018

@june29 さん
そういうことでしたか、私がミスリードしておりました…申し訳ありません。
基本的な並行開発の手法なのですが、この場合はブランチをプラクティスの数だけ切って1個ずつコミット→プルリクエスト、という形になるのでしょうか。
できれば機能ごとにPRを出していきたいです。

udzura
udzura reviewed Oct 17, 2018
View reviewed changes
@tosite tosite changed the title 00.文字列の逆順 言語処理100本ノックの回答です。Rubyの構文・記述などに不備がないかチェックをお願いいたします。 Oct 17, 2018
@tosite
Copy link
Collaborator Author

tosite commented Oct 17, 2018

@udzura さんのご指導によりPRタイトルを変更しました。
記述に不備がないかのチェックを改めてお願いいたします。

@udzura
Copy link
Contributor

udzura commented Oct 17, 2018

どういうPRかわかるようになって良さそうです！

Rubyの構文・記述などに不備がないかチェックをお願いいたします。

これを含めてdecsription（PRの最初の項目）も更新するとベターです 😃
僕なら例えばこうするかもしれません。
（多くのプロジェクトでは、PRのテンプレートがあるので、それを埋めていくのが良いと思います）

## レビューして欲しい人
* @udzura, @litencatt, @june29 

## レビュー観点
* Rubyの構文・記述などに不備がないかチェックをお願いいたします。

## マージ条件
* 一通りのレビューを受け、対応したらマージします。

udzura
udzura reviewed Oct 17, 2018
View reviewed changes
Copy link
Contributor

@udzura udzura left a comment

Choose a reason for hiding this comment

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

学びを即活かしていてめっちゃいい...

@tosite
Copy link
Collaborator Author

tosite commented Oct 18, 2018

@udzura さん
Descriptionの修正終わりました。
改めてご確認お願いいたします。

@udzura
Copy link
Contributor

udzura commented Oct 18, 2018

よーし！
僕の方では今週いっぱいをめどにまたみておきます！　が、rubocopも入ったしそこまでツッコミどころなさそう。

@june29
Copy link

june29 commented Oct 18, 2018

タイトルの

Rubyの構文・記述などに不備がないかチェックをお願いいたします。

は「この Pull Request はなにを成すのか」の情報ではないから省いちゃってよさそう 👀

@june29
Copy link

june29 commented Oct 18, 2018

「言語処理100本ノックとはなにか？」は自明ではないと思うので、Pull Request の概要欄にhttp://www.cl.ecei.tohoku.ac.jp/nlp100/ の URL を置いておくのはいかがでしょ？

それと、現時点で 100 本のうちの 09 までのコードしかないように見えますが、この Pull Request ではどこまでを扱うつもりですか？それがわかればまともにレビューできそうです 💨

@tosite tosite changed the title 言語処理100本ノックの回答です。Rubyの構文・記述などに不備がないかチェックをお願いいたします。 言語処理100本ノックのうち、00〜09）の回答です。 Oct 18, 2018
@tosite tosite changed the title 言語処理100本ノックのうち、00〜09）の回答です。 言語処理100本ノックのうち、00〜09までの回答です。 Oct 18, 2018
@tosite tosite requested a review from litencatt October 18, 2018 08:40
@tosite
Copy link
Collaborator Author

tosite commented Oct 18, 2018

@june29 さん
ご指摘ありがとうございます！
早速修正しておきました。

litencatt
litencatt reviewed Oct 18, 2018
View reviewed changes
litencatt
litencatt reviewed Oct 18, 2018
View reviewed changes
@june29
Copy link

june29 commented Oct 18, 2018

タイトルと概要の更新、ありがとうございます！ 😊

これで、仮にぼくがいま初めてこの Pull Request を見にきたとしても、まともにレビューできるだけの情報が揃ったな〜と思います。わーいわーい。

@tosite
Copy link
Collaborator Author

tosite commented Oct 18, 2018

丁寧なご指導ありがとうございました！
少しずつですがP/Rの出し方が分かってきたように思います。
伝えるって大事ですね…！

わーいわーい！

udzura
udzura reviewed Oct 19, 2018
View reviewed changes
udzura
udzura reviewed Oct 19, 2018
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@litencatt litencatt litencatt left review comments

+2 more reviewers

@june29 june29 june29 left review comments

@udzura udzura udzura left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tosite @udzura @june29 @komagata @litencatt