HewlettPackard / lustre_exporter

Prometheus exporter for use with the Lustre parallel filesystem
Apache License 2.0
36 stars 51 forks source link

Makefile improvements #61

Closed mjtrangoni closed 7 years ago

mjtrangoni commented 7 years ago

I made some small aesthetic changes to the Makefile and fix also a vet warning,

>> formatting code
>> vetting code
lustre_exporter.go:60: range variable c captured by func literal
exit status 1
Makefile:35: recipe for target 'vet' failed
make: *** [vet] Error 1

In addition I create a (.gitignore)(gitignore) file to exclude *~ and the binary created lustre_exporter.

I hope you agree with the changes. Thanks!

joehandzik commented 7 years ago

@mjtrangoni This all looks good to me, though your third commit (Fix range variable c...) probably makes sense as a separate PR. Can you cherry-pick it out of here and into a new branch, and spin off a second PR to cover it? Otherwise, I'll let @roclark take a look, and we'll merge whenever you move your third commit to a new PR.

mjtrangoni commented 7 years ago

@joehandzik I made some more improvements, and drop the Fix change as you requested.

Curiously I am getting no error with Travis-CI, and local I am having this,

$ make 
>> formatting code
>> vetting code
lustre_exporter.go:60: range variable c captured by func literal
exit status 1
Makefile:34: recipe for target 'vet' failed
make: *** [vet] Error 1

Any ideas why it doesn't look the same?

joehandzik commented 7 years ago

@mjtrangoni Not sure why your result would be different, I'm trying 'go vet' against master and it's failing with the same error that you get too. If you spin off a PR to fix that issue though, I can merge it and then you can rebase the change into this PR, which should fix the problem locally too.

mjtrangoni commented 7 years ago

@joehandzik done. See #65

joehandzik commented 7 years ago

65 merged, rebase when you have a chance and confirm that your local builds are working again!

mjtrangoni commented 7 years ago

It works!

$ make
>> formatting code
>> vetting code
>> building binaries
 >   lustre_exporter
>> running tests
?       github.com/HewlettPackard/lustre_exporter   [no test files]
?       github.com/HewlettPackard/lustre_exporter/sources   [no test files]
joehandzik commented 7 years ago

@mjtrangoni Awesome! I'll wait for an ack from @roclark then pull in this PR.