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.

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

CartでSAL Parcelが選択できるはずの条件で選択肢に出ない場合がある問題の修正 #2

Merged
merged 11 commits into from Sep 15, 2015

Conversation

nezumi650
Copy link
Contributor

@nezumi650 nezumi650 added the bug label Sep 10, 2015
@nezumi650 nezumi650 changed the title CartでSAL Parcelが選択できるはずの条件で選択肢に出ない場合がある問題の修正 [WIP]CartでSAL Parcelが選択できるはずの条件で選択肢に出ない場合がある問題の修正 Sep 10, 2015
@nezumi650 nezumi650 changed the title [WIP]CartでSAL Parcelが選択できるはずの条件で選択肢に出ない場合がある問題の修正 CartでSAL Parcelが選択できるはずの条件で選択肢に出ない場合がある問題の修正 Sep 11, 2015
@naoina
Copy link
Collaborator

naoina commented Sep 14, 2015

OKだと思います。

@pchw 続きよろしくお願いします。

@nezumi650
Copy link
Contributor Author

@pchw
822ed7d で追加していただいたテストケースも通る様になりました。ご確認をお願いします!

@nezumi650
Copy link
Contributor Author

@pchw
https://github.com/tokyootakumode/calc-box/pull/2/files#diff-640d35de68e05bf123eb74894ebb8099R113
ここの事ですよね?

すみません!変数名がいまいちなので、変数名を修正します。

ちなみに、この変数に格納したいのは Parcel の 2番目に長い辺であってます。
Parcel の一番長い辺はすでに箱のどの辺に合わせるかが決まっているので、この段階ではとくに考慮していません。「この処理内での、Parcel の一番長い辺」という意味で変数名をつけました。(「二番目に長い辺」という単語がパッと出てこなかったので…)

@nezumi650
Copy link
Contributor Author

@pchw 修正しました!再レビューをお願いいたします! 🙇

@pchw
Copy link
Member

pchw commented Sep 15, 2015

@nezumi650 大丈夫っぽい
LGTM
テストのデバッグ用コメントは消しちゃっていいかなー

@nezumi650
Copy link
Contributor Author

@pchw @naoina レビューありがとうございました!

テストのデバッグ用コメントは消しちゃっていいかなー

削除しました!この形でリリースしようと思います。

nezumi650 added a commit that referenced this pull request Sep 15, 2015
CartでSAL Parcelが選択できるはずの条件で選択肢に出ない場合がある問題の修正
@nezumi650 nezumi650 merged commit 2818a30 into master Sep 15, 2015
@nezumi650 nezumi650 deleted the 14237-fix-pushParcel branch September 15, 2015 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants