search

TheFox / flickr-cli

A command-line interface to Flickr. Upload and download photos, photo sets, directories via shell.
https://fox21.at
25 stars 9 forks source link

Display an error message if upload fails #41

Open tacman opened 6 years ago

tacman commented 6 years ago

There was no error message if the endpoint returned a valid SimpleXML object but it contained an error message. This displays one now (for example, a permissions error).

TheFox commented 6 years ago

This is a little bit too much logging. There is already a logging call. See Line 425 in your file:

$this->getLogger()->info(sprintf('[file] status: %s - ID %s', $logLine, $photoId));

There is already a if ($successful) check. I want to keep it simple.

Therefore I have to reject this PR. But thanks for your will to improve Flickr CLI.

tacman commented 6 years ago

Could you add the error message to the log? The problem is that if the REST returns an error, there's no way to know what the error is, it simply says FAIL.

On Mon, Feb 19, 2018 at 9:31 AM, Christian Mayer notifications@github.com wrote:

This is a little bit too much logging. There is already a logging call. See Line 425 https://github.com/tacman/flickr-cli/blob/9f1102c4ca843b79187691ac3ee96eed637e9d0f/src/TheFox/FlickrCli/Command/UploadCommand.php#L324 in your file:

$this->getLogger()->info(sprintf('[file] status: %s - ID %s', $logLine, $photoId));

There is already a if ($successful) check. I want to keep it simple.

Therefore I have to reject this PR. But thanks for your will to improve Flickr CLI.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/TheFox/flickr-cli/pull/41#issuecomment-366709733, or mute the thread https://github.com/notifications/unsubscribe-auth/AAl0QdEvee0VjDwqE6K3jLs2d8-Bj34aks5tWYXagaJpZM4SKCed .

tacman commented 6 years ago

Also, it should be logger->error, not logger->info, since it is definitely an error and should be tagged as such (that is, you don't need to turn on verbose to see it).

On Mon, Feb 19, 2018 at 9:41 AM, Tac Tacelosky tacman@gmail.com wrote:

Could you add the error message to the log? The problem is that if the REST returns an error, there's no way to know what the error is, it simply says FAIL.

On Mon, Feb 19, 2018 at 9:31 AM, Christian Mayer <notifications@github.com

wrote:

This is a little bit too much logging. There is already a logging call. See Line 425 https://github.com/tacman/flickr-cli/blob/9f1102c4ca843b79187691ac3ee96eed637e9d0f/src/TheFox/FlickrCli/Command/UploadCommand.php#L324 in your file:

$this->getLogger()->info(sprintf('[file] status: %s - ID %s', $logLine, $photoId));

There is already a if ($successful) check. I want to keep it simple.

Therefore I have to reject this PR. But thanks for your will to improve Flickr CLI.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/TheFox/flickr-cli/pull/41#issuecomment-366709733, or mute the thread https://github.com/notifications/unsubscribe-auth/AAl0QdEvee0VjDwqE6K3jLs2d8-Bj34aks5tWYXagaJpZM4SKCed .

TheFox commented 6 years ago

Good point. I'll to that.