gerlowskija commented on code in PR #4731:
URL: https://github.com/apache/solr/pull/4731#discussion_r3791646871
##########
solr/core/src/test/org/apache/solr/core/AlternateDirectoryTest.java:
##########
@@ -33,7 +33,7 @@ public static void beforeClass() throws Exception {
}
public void testAltDirectoryUsed() {
- assertQ(req("q", "*:*", "qt", "/select"));
+ assertQ(reqWithPath("/select", "q", "*:*"));
Review Comment:
Sorry - I caught that suggestion in your last review and did consider it.
Since it was a "small matter of taste" I didn't respond, but it would've been
more considerate had I done so - my bad.
I did try out the heavier use of `params(..)` in a few places in the
previous PR. But in practice it didn't seem to me that there was much
confusion that the extra `params(...)` helped alleviate. In other words: when
I as a reader saw `req("q", "*:*", "fl", "id,score", ...)`, the types and
semantics seemed sufficiently clear without the more explicit `params(...)`
formulation. That's my personal taste I guess.
If you feel more strongly, lmk. Otherwise I'll leave the pattern as-is
going forward.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
