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

Windows porting #8

Merged
merged 15 commits into from Jun 5, 2014

Conversation

Projects
None yet
3 participants
@mattn
Contributor

mattn commented May 22, 2014

Windows にポーティングしました。

  • conf ファイルの位置を個々のアーキテクチャで保持し、それをデフォルト値に。
  • Windows 向け spec/metrics 処理を追加。

related issue #3

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn May 22, 2014

Contributor

幾つかテストを無効にしていますが、これは Linux と同じテストが出来ないので(NIC名とか)対処を検討中です。

Contributor

mattn commented May 22, 2014

幾つかテストを無効にしていますが、これは Linux と同じテストが出来ないので(NIC名とか)対処を検討中です。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn May 22, 2014

Contributor

尚、Windows のドライブマウント先は C: の様になるので、おそらく現状 / で始まっていないといけないサーバ側を直して頂く必要があります。直り次第、こちらのTODO対処を行います。

https://github.com/mattn/mackerel-agent/blob/windows_porting/spec/windows/filesystem.go#L81-L91

Contributor

mattn commented May 22, 2014

尚、Windows のドライブマウント先は C: の様になるので、おそらく現状 / で始まっていないといけないサーバ側を直して頂く必要があります。直り次第、こちらのTODO対処を行います。

https://github.com/mattn/mackerel-agent/blob/windows_porting/spec/windows/filesystem.go#L81-L91

@hakobe

This comment has been minimized.

Show comment
Hide comment
@hakobe

hakobe May 22, 2014

Contributor

早速、ありがとうございます!

幾つかテストを無効にしていますが、これは Linux と同じテストが出来ないので(NIC名とか)対処を検討中です。

等価の内容であれば、多少テストが違っていても問題ないと思っています。

尚、Windows のドライブマウント先は C: の様になるので、おそらく現状 / で始まっていないといけないサーバ側を直して頂く必要があります。直り次第、こちらのTODO対処を行います。

再度確認してみます。

importされているhttp://github.com/mackerelio/mackerel-agent/util/windowsがリポジトリに存在しないように見えます。コミット忘れされていませんか?

Contributor

hakobe commented May 22, 2014

早速、ありがとうございます!

幾つかテストを無効にしていますが、これは Linux と同じテストが出来ないので(NIC名とか)対処を検討中です。

等価の内容であれば、多少テストが違っていても問題ないと思っています。

尚、Windows のドライブマウント先は C: の様になるので、おそらく現状 / で始まっていないといけないサーバ側を直して頂く必要があります。直り次第、こちらのTODO対処を行います。

再度確認してみます。

importされているhttp://github.com/mackerelio/mackerel-agent/util/windowsがリポジトリに存在しないように見えます。コミット忘れされていませんか?

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn May 22, 2014

Contributor

おっと!

Contributor

mattn commented May 22, 2014

おっと!

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn May 22, 2014

Contributor

すいませんすいません

Contributor

mattn commented May 22, 2014

すいませんすいません

Show outdated Hide outdated util/windows/util.go
Show outdated Hide outdated util/windows/util.go
@hakobe

This comment has been minimized.

Show comment
Hide comment
@hakobe

hakobe May 23, 2014

Contributor

@mattn ありがとうございます!

少しコメントさせていただいたので、検討していただけますと、ありがたいです。こちらで修正してしまうのでも、かまわないです。

修正後にぜひマージさせていただきたいですが、手元でのWindows検証環境がすぐに用意できないのと、検証期間の都合で、数日中の行う予定の次回リリース以降にマージさせていただくことになると思います。

Contributor

hakobe commented May 23, 2014

@mattn ありがとうございます!

少しコメントさせていただいたので、検討していただけますと、ありがたいです。こちらで修正してしまうのでも、かまわないです。

修正後にぜひマージさせていただきたいですが、手元でのWindows検証環境がすぐに用意できないのと、検証期間の都合で、数日中の行う予定の次回リリース以降にマージさせていただくことになると思います。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn May 23, 2014

Contributor

そうですね。util_windows.go は、もし util_linux.go が出来た時に同じインタフェース構成になっている事を開発者が期待してしまうので、別パッケージとします。
また util/windows/util.goutil/windows/windows.go も開発者が勘違いしそうなので util/windows/windows_api.go とします。(パッケージ名は windows)
なお当初 util.CreateQuery の様に、全てに util. を付けようかと思いましたが、キャストの為に util.XXX() とする必要があり、かつ引数の数が多い為に逆に可読性を失ってしまったので . import しました。

後程対応します。

Contributor

mattn commented May 23, 2014

そうですね。util_windows.go は、もし util_linux.go が出来た時に同じインタフェース構成になっている事を開発者が期待してしまうので、別パッケージとします。
また util/windows/util.goutil/windows/windows.go も開発者が勘違いしそうなので util/windows/windows_api.go とします。(パッケージ名は windows)
なお当初 util.CreateQuery の様に、全てに util. を付けようかと思いましたが、キャストの為に util.XXX() とする必要があり、かつ引数の数が多い為に逆に可読性を失ってしまったので . import しました。

後程対応します。

@hakobe

This comment has been minimized.

Show comment
Hide comment
@hakobe

hakobe May 23, 2014

Contributor

ありがとうございます! util/windows/windows_api.go に build constraint も付与していただくと嬉しいです。

Contributor

hakobe commented May 23, 2014

ありがとうございます! util/windows/windows_api.go に build constraint も付与していただくと嬉しいです。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn May 23, 2014

Contributor

すみません!

Contributor

mattn commented May 23, 2014

すみません!

@hakobe

This comment has been minimized.

Show comment
Hide comment
@hakobe

hakobe May 23, 2014

Contributor

やったー!ありがとうございます。 動作確認後mergeされるとおもいます!確認のなかで質問がでてきたら、またココで議論させてください。

Contributor

hakobe commented May 23, 2014

やったー!ありがとうございます。 動作確認後mergeされるとおもいます!確認のなかで質問がでてきたら、またココで議論させてください。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn May 23, 2014

Contributor

はい!おねがいします!

Contributor

mattn commented May 23, 2014

はい!おねがいします!

@skozawa

This comment has been minimized.

Show comment
Hide comment
@skozawa

skozawa Jun 4, 2014

Contributor

@hakobe に代わり、windows環境で動作確認させていただきました。ラインコメントしましたが、以下の3点についてご対応いただけると助かります、よろしくお願いします > @mattn さん

  • generatorsにnilを含めないように
  • percent_used の修正
  • windows drive path 対応
Contributor

skozawa commented Jun 4, 2014

@hakobe に代わり、windows環境で動作確認させていただきました。ラインコメントしましたが、以下の3点についてご対応いただけると助かります、よろしくお願いします > @mattn さん

  • generatorsにnilを含めないように
  • percent_used の修正
  • windows drive path 対応
@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Jun 4, 2014

Contributor

レビューありがとうございます。助かります。
現在修正は終わり、ランニングさせていますので、ディスク容量について正しいグラフが取れ次第 push します。

Contributor

mattn commented Jun 4, 2014

レビューありがとうございます。助かります。
現在修正は終わり、ランニングさせていますので、ディスク容量について正しいグラフが取れ次第 push します。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Jun 4, 2014

Contributor

修正しました。totalNumberOfFreeBytes は必要ないので取得しない様にしました。

Contributor

mattn commented Jun 4, 2014

修正しました。totalNumberOfFreeBytes は必要ないので取得しない様にしました。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Jun 4, 2014

Contributor

お手数おかけします。
修正しました。

Contributor

mattn commented Jun 4, 2014

お手数おかけします。
修正しました。

@skozawa

This comment has been minimized.

Show comment
Hide comment
@skozawa

skozawa Jun 5, 2014

Contributor

修正ありがとうございます!!
LGTM!!

Contributor

skozawa commented Jun 5, 2014

修正ありがとうございます!!
LGTM!!

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Jun 5, 2014

Contributor

Contributor

mattn commented Jun 5, 2014

@skozawa

This comment has been minimized.

Show comment
Hide comment
@skozawa

skozawa Jun 5, 2014

Contributor

windows対応ありがとうございました、マージします!!

ただし、公式サポートは http://help-ja.mackerel.io/entry/overview の対応環境にある通りで、当面windows環境での動作は公式サポートの対象外となることをご了承ください。

Contributor

skozawa commented Jun 5, 2014

windows対応ありがとうございました、マージします!!

ただし、公式サポートは http://help-ja.mackerel.io/entry/overview の対応環境にある通りで、当面windows環境での動作は公式サポートの対象外となることをご了承ください。

skozawa added a commit that referenced this pull request Jun 5, 2014

@skozawa skozawa merged commit 88b1a22 into mackerelio:master Jun 5, 2014

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Jun 5, 2014

Contributor

はい!ありがとうございます。

Contributor

mattn commented Jun 5, 2014

はい!ありがとうございます。

daiksy added a commit that referenced this pull request Oct 20, 2016

Merge pull request #8 from hatena/delete_plugins_on_pluginlist
wix/pluginlistからmackerel-agent-plugins を消した

@stanaka stanaka referenced this pull request Oct 27, 2016

Merged

Release version 0.37.0 #290

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