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

Add upload capability #8

Closed
wants to merge 3 commits into
base: master
from

Conversation

4 participants
@twooster

twooster commented Mar 22, 2018

Could be a nice addition :)

@claudiodangelis

This comment has been minimized.

Owner

claudiodangelis commented Mar 22, 2018

This looks extremely interesting!

I will review this proposal in a few days, thank you!

@djm158

This comment has been minimized.

djm158 commented Mar 23, 2018

As @bvaughn has noted in other issues, @cIaudiodangeIis is a troll account, do not confuse them with @claudiodangelis.

Please report the troll account as abusive.

@twooster

This comment has been minimized.

twooster commented Mar 23, 2018

The guy was a troll but it's not entirely wrong; first time writing go. Cleaned up a bit.

@twooster twooster force-pushed the twooster:add-upload-capability branch from 20153f6 to 4a8af6d Mar 23, 2018

log.Fatalln(err)
// Check how many arguments are passed
if *uploadFlag == true {

This comment has been minimized.

@barryz

barryz Mar 23, 2018

just use if *uploadFlag {...}

io.Copy(f, upload)
upload.Close()
f.Close()

This comment has been minimized.

@barryz

barryz Mar 23, 2018

should use defer statement

This comment has been minimized.

@twooster

twooster Mar 23, 2018

I used to do that, but after reading the documentation for os.Exit
(following the convention of the other handler, here), I decided it was
better to close them explicitly. (From doc'm: The program terminates
immediately; deferred functions are not run.) I would expect the explicit
ordered Close to flush and ensure all data has been appropriately written.

@claudiodangelis

This comment has been minimized.

Owner

claudiodangelis commented Mar 23, 2018

@twooster I don't think your code is ugly and badly written, and I really appreciate the time and effort you and @barryz are spending on this.

@claudiodangelis

This comment has been minimized.

Owner

claudiodangelis commented Mar 31, 2018

@twooster, @barryz, I will put this proposal "on hold" until other pending PRs are merged.

@claudiodangelis

This comment has been minimized.

Owner

claudiodangelis commented May 17, 2018

Hello @twooster, sorry for the huge delay, I think we are ready to resume working. I'm reviewing the conflicts now.

@claudiodangelis

This comment has been minimized.

Owner

claudiodangelis commented Aug 16, 2018

Superseded by: #74

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