pgj commented on code in PR #5008:
URL: https://github.com/apache/couchdb/pull/5008#discussion_r1527471183
##########
src/couch_replicator/test/eunit/couch_replicator_scheduler_docs_tests.erl:
##########
@@ -133,9 +133,6 @@ t_scheduler_docs_total_rows({_Ctx, {RepDb, Source,
Target}}) ->
case req(get, SchedulerDocsUrl) of
{200, #{<<"docs">> := [_ | _]} = Decoded} ->
Decoded;
- {_, #{<<"error">> := Error, <<"reason">> := Reason}} ->
Review Comment:
> If we get an error or anything else it should fail immediately with a bad
match.
Yes, that is what #5007 is trying to implement. This focuses on
intercepting errors in the response, explore the context, report it to the user
(surface it), and then raise an error (make the test fail). But I can see that
treating anything else than the happy path as error is probably a simpler way
to express the same. Do you think that the resulting "bad match" in your case
will give enough information for the developer?
> In general, or at least in Apache CouchDB, debug logs look a bit out of
place. [..] Otherwise after a few years nobody knows why they are there or if
they are still needed.
Well, with this debug logging of mine we found a bug already. And one could
still "blame" that part of the code to learn about [the
motivation](https://github.com/apache/couchdb/commit/d7188ba8dea81739e8535b92ede1c6613bb598f8)
:-)
Either way it goes, I do not have a strong opinion about the related style
or the habits. All I would consider meaningful is to avoid masking errors by
timeouts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
