You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/01/27 13:18:06 UTC
[GitHub] [solr] gerlowskija opened a new pull request #570: Move all '*Payload' classes inside their API counterparts
gerlowskija opened a new pull request #570:
URL: https://github.com/apache/solr/pull/570
# Description
Mike Drob requested in a recent PR (#565), that all v2 "Payload" classes be relocated under their counterpart "API" classes.
# Solution
This PR introduces this refactor, as promised.
# Tests
Existing v2 API or conversion tests should all still pass - this PR doesn't introduce anything net-new test-wise, since it's a pure refactor.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #570: Move all '*Payload' classes inside their API counterparts
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on pull request #570:
URL: https://github.com/apache/solr/pull/570#issuecomment-1023210384
FYI @madrob - here's the refactor of those 'Payload' classes, as requested.
There's still a few beans missing from this PR. A few (Package, PluginMeta) were named and used a little differently than I'm used to, and I hope to get to them when I get a bit more time.
Two others (DeleteBackupPayload, ListBackupPayload) are already used to implement SolrRequest classes in SolrJ (the eventual other use of these payload classes that I mentioned [here](https://github.com/apache/solr/pull/565#issuecomment-1021504830) and [here](https://issues.apache.org/jira/browse/SOLR-15187).) I must've done these two as models when adding them in SIP-12 and forgotten about it. Anyway I _don't_ plan on moving these under their API counterparts, as it'd require undoing the SolrRequest implementations.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on pull request #570: Move all '*Payload' classes inside their API counterparts
Posted by GitBox <gi...@apache.org>.
madrob commented on pull request #570:
URL: https://github.com/apache/solr/pull/570#issuecomment-1023537786
So after seeing your pointer to the DeleteBackupPayload class, I understand more of the intent, and I no longer think this is a worthwhile change. I'm sorry that you ended up going through the large refactor to illustrate this, and agree with you that we should be moving the other way.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #570: Move all '*Payload' classes inside their API counterparts
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #570:
URL: https://github.com/apache/solr/pull/570#discussion_r793731166
##########
File path: solr/core/src/java/org/apache/solr/handler/admin/api/ReloadCollectionAPI.java
##########
@@ -63,4 +62,9 @@ public void reloadCollection(PayloadObj<ReloadCollectionPayload> obj) throws Exc
collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}
+
+ public class ReloadCollectionPayload implements ReflectMapWriter {
Review comment:
*ClassCanBeStatic:* Inner class is non-static but does not reference enclosing class [(details)](https://errorprone.info/bugpattern/ClassCanBeStatic)
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org