flickr-cli icon indicating copy to clipboard operation
flickr-cli copied to clipboard

Display an error message if upload fails

Open tacman opened this issue 7 years ago • 4 comments

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).

tacman avatar Feb 19 '18 04:02 tacman

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.

TheFox avatar Feb 19 '18 14:02 TheFox

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 [email protected] 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 avatar Feb 19 '18 14:02 tacman

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 [email protected] 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 <[email protected]

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 avatar Feb 19 '18 14:02 tacman

Good point. I'll to that.

TheFox avatar Feb 19 '18 14:02 TheFox