Skip to content

Conversation

kotamanegi
Copy link
Contributor

kotamanegiです。

Windows環境でインストールしようと思った所、exeが生成されなかった為コマンドが実行できませんでした。
そのため、Windowsの場合exeが生成されるようにコードを修正しました。

始めてのプルリクなので作法が全く分かっていないのですが、アドバイスなどあればよろしくお願いします。

@codecov-io
Copy link

codecov-io commented Mar 4, 2019

Codecov Report

Merging #122 into master will decrease coverage by 1.84%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #122      +/-   ##
==========================================
- Coverage   90.48%   88.64%   -1.85%     
==========================================
  Files          48       49       +1     
  Lines        2165     2210      +45     
==========================================
  Hits         1959     1959              
- Misses        206      251      +45
Impacted Files Coverage Δ
atcodertools/atcoder_tools.py 0% <0%> (ø)

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 53cb92b...1480381. Read the comment docs.

@kyuridenamida
Copy link
Owner

kotamanegiさん! PRありがとうございます(初PRめでたい!)。 Windowsユーザーはやや少なめだったので助かります。
初めてということを念頭に入れつつレビューしますね!
今やって頂いている通り、まずは自動CIが通るようにしてもらうことが第一歩ですね。

@kyuridenamida kyuridenamida changed the title fixed for compatibility with Windows (kotamanegi) Fix compatibility problem on Windows Mar 4, 2019
@kyuridenamida
Copy link
Owner

カバレッジは×になってますが、今回無視してくれていいです。travis-ciがOKならOKです。ちょっと

ちょっと環境を再現して手元でも試したいので、使っているpipのバージョンとpythonのバージョンを教えてくれますか。

@kotamanegi
Copy link
Contributor Author

なるほど、分かりました。
環境はWindows:Python 3.7.2 pip19.0.3 linux:Python 3.6 pip:9.0.1 でやってます。
macやlinux環境下できちんと動くかはあまり検証出来てないので、よろしくお願いします。

@kyuridenamida
Copy link
Owner

Python 3.7.2 / pip 18.1で動作することを確認しました。MANIFESTだけ微修正してくれたらマージします。

Copy link
Owner

@kyuridenamida kyuridenamida left a comment

Choose a reason for hiding this comment

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

ところでsetup.py全体にdiffが発生してる理由ってわかりますか?わかるならdiff箇所だけ現れるようにしてほしいです。わからなければそのままで。

@kotamanegi
Copy link
Contributor Author

やばいです、コミットメッセージ間違ってます

@kotamanegi
Copy link
Contributor Author

コミットメッセージを修正しました。
setup.py全体にdiffが発生しているのは、おそらく改行コードが違っていたからだと思います。
アドバイス本当にありがとうございました。

@kyuridenamida
Copy link
Owner

いえいえ、迅速な修正ありがとうございました。マージしますね。

@kyuridenamida
Copy link
Owner

実際この変更はWindowsユーザーにはかなり嬉しいと思うので助かります

@kyuridenamida kyuridenamida merged commit a7bf8ae into kyuridenamida:master Mar 5, 2019
@kyuridenamida kyuridenamida added this to the 1.1.3 milestone Mar 5, 2019
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.

3 participants