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

don't open log file before truncate file #906

Closed
wants to merge 2 commits into from

Conversation

mei-rune
Copy link
Contributor

@mei-rune mei-rune commented Sep 4, 2014

failed to trancate file while file has opened on some filesystem(on the win32), don't open log file before trancate file.

I hope influxdb can be ported to win32 platform, I have more pull request, which is one of。

#839 is duplcate, I close it

if err != nil {
func newLog(logFileName string, config *configuration.Configuration) (*log, error) {
size, err := checkAndRepairLogFile(logFileName)
if nil != err {
Copy link
Contributor

Choose a reason for hiding this comment

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

While this is an admirable programming style to prevent the classic bug of assignment instead of comparison, it is not possible in Go to make this mistake. The compiler won't let you do it.

http://play.golang.org/p/0aO2Gqwupe
http://play.golang.org/p/YoYqojbVU5

I suggest you follow the standard style.

@mei-rune
Copy link
Contributor Author

mei-rune commented Sep 5, 2014

I have modified according to your suggestions

jvshahid pushed a commit that referenced this pull request Sep 8, 2014
@jvshahid jvshahid force-pushed the master branch 2 times, most recently from 987efd5 to 5e9750f Compare October 20, 2014 22:37
@toddboom toddboom changed the title don't open log file before trancate file don't open log file before truncate file Oct 23, 2014
@toddboom
Copy link
Contributor

I think the changes for v0.9.0 are so significant that this won't end up getting merged in. Thanks for the hard work, @runner-mei!

@toddboom toddboom closed this Dec 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants