Skip to content

Conversation

kotamanegi
Copy link
Contributor

@kotamanegi kotamanegi commented Mar 6, 2019

修正点は以下の点です。

  • Windows環境下において、文字の色が変わらない問題を解決しました。

お忙しい中すみませんが、よろしくお願いします。

@codecov-io
Copy link

codecov-io commented Mar 6, 2019

Codecov Report

Merging #127 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #127      +/-   ##
==========================================
+ Coverage   88.65%   88.65%   +<.01%     
==========================================
  Files          49       49              
  Lines        2212     2213       +1     
==========================================
+ Hits         1961     1962       +1     
  Misses        251      251
Impacted Files Coverage Δ
atcodertools/tools/utils.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c718b9...137199d. Read the comment docs.

@kyuridenamida
Copy link
Owner

ありがとうございます!URLもですが、特に色のやつかなりありがたいです!
issueをまとめることに関しての指南ですが、完全に独立したチェンジの場合、まとめることによる開発オーバヘッドよりも、分離することによるメリット(問題ない部分から優先的にマージを行える、議論が混ざらない、後からどういう変更があったか分かりやすい、CHANGELOGでPR毎に何したかを書きやすい...etc)があるので、実は作業量単位ではなく意味単位で分けるのがおすすめです。最初は面倒くさく感じるかもしれませんが・・・。

@kyuridenamida
Copy link
Owner

なので、出来ればコードの色変更以外のコードは戻して(コミット履歴改変してpush -fでも構いません)、#118に関する新URLのチェンジは別PRに分けてくれると嬉しいです。コードの色変更に関しては多分問題なくて、new_urlの部分に関してはいくつか変更をお願いしたい部分があるので、その部分に関するコメントは後でさせてください。

@kotamanegi
Copy link
Contributor Author

了解です。
issue#118の修正については別PRに分離します。

@kyuridenamida kyuridenamida changed the title Fix minor issues Support coloring functionality on Windows Mar 8, 2019
@kyuridenamida
Copy link
Owner

ありがとうございます。よさそうなのでマージします

@kyuridenamida kyuridenamida merged commit 5776282 into kyuridenamida:master Mar 8, 2019
kyuridenamida added a commit that referenced this pull request Mar 8, 2019
kyuridenamida added a commit that referenced this pull request Mar 8, 2019
@kyuridenamida
Copy link
Owner

実はlinuxで動作確認をしたら色が消失していたので、一旦revertしました。すみません。

@kotamanegi
Copy link
Contributor Author

!?!?
ちょっと確認してみます。

@kyuridenamida
Copy link
Owner

手元で試した結果convert=Trueが原因かなあとは思うんですが、これがないとWindowsでは動作しませんか?

@kotamanegi
Copy link
Contributor Author

そこを消してもWindows環境下での動作には影響がありませんでした。
WSL環境が今バグっているのでLinux環境で検証してもらえますか?

@kyuridenamida
Copy link
Owner

linuxはいい感じでした

@kotamanegi
Copy link
Contributor Author

大丈夫そうなのでプルリクをもう一度送ります。

@kyuridenamida
Copy link
Owner

すみません。既にマージされたプルリク、再マージができない仕様で泣いちゃった。masterから派生した新しいプルリク作ってもらえますか。それはそれとして大学合格おめでとうございます。

@kotamanegi
Copy link
Contributor Author

もう反映されていると思います。
(新しく作っておいたプルリクがすでにマージされているので)
後輩として恥ずかしくないように頑張ります。
ありがとうございました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants