dsmiley commented on code in PR #4080:
URL: https://github.com/apache/solr/pull/4080#discussion_r2762382739
##########
solr/packaging/test/test_rolling_upgrade.bats:
##########
Review Comment:
out-of-scope
##########
solr/core/src/java/org/apache/solr/response/ResponseWritersRegistry.java:
##########
@@ -46,21 +48,14 @@ private ResponseWritersRegistry() {
PrometheusResponseWriter prometheusWriter = new PrometheusResponseWriter();
BUILTIN_WRITERS =
- Map.of(
- CommonParams.JAVABIN,
- new JavaBinResponseWriter(),
- CommonParams.JSON,
- jsonWriter,
- "standard",
- jsonWriter, // Alias for JSON
- "xml",
- new XMLResponseWriter(),
- PROMETHEUS_METRICS_WT,
- prometheusWriter,
- OPEN_METRICS_WT,
- prometheusWriter,
- ReplicationAPIBase.FILE_STREAM,
- new FileStreamResponseWriter());
+ Map.ofEntries(
Review Comment:
nice and tidy :-)
##########
solr/test-framework/src/java/org/apache/solr/util/TestHarness.java:
##########
@@ -419,6 +419,7 @@ public LocalRequestFactory() {}
@SuppressWarnings({"unchecked"})
public LocalSolrQueryRequest makeRequest(String... q) {
if (q.length == 1) {
+ args.computeIfAbsent("wt", k -> "xml");
Review Comment:
hmm... how did this end up failing without this?
##########
solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java:
##########
Review Comment:
what's going on here?
note: commented code is typically avoided
##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -727,7 +727,7 @@ private void handleAdminRequest() throws IOException {
protected void logAndFlushAdminRequest(SolrQueryResponse solrResp) throws
IOException {
if (solrResp.getToLog().size() > 0) {
- // has to come second and in it's own if to keep ./gradlew check happy.
+ // has to come second and in its own "if" to keep ./gradlew check happy.
Review Comment:
minor: I prefer that we don't change source files for out-of-scope reasons
that we aren't already touching for in-scope reasons.
##########
solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java:
##########
Review Comment:
Are these changes in-scope?
##########
changelog/unreleased/SOLR-18085.yml:
##########
@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Removed the wt=standard concept that was used internally by Solr. It
was redundent with json writer type that was used as well.
Review Comment:
why reference json/redundancy? It doesn't matter what the default is.
What's pertinent is that we need not have an alias for the default. The
default is the default, whatever that may be (doesn't matter).
--
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]
