athrog commented on pull request #120: URL: https://github.com/apache/solr/pull/120#issuecomment-877374795
> Some first thoughts: > > * Are y'all planning on renaming the contrib to s3 from blob? It seems like a logical choice Yes, renaming the module to `s3-repository` will be in my next commit. > * I mentioned this in a comment, but the errors that are given back are generally not very helpful in fixing problems. I think a lot of them stem from using stuff like Preconditions, which pass a generic error back to the User and doesn't tell them what they need to do to fix the problem. I agree with @madrob , we shouldn't have any Preconditions in this code. Same for `Objects.requireNonNull`. Sounds like there are two separate issues here: (1) usage of Preconditions/Objects for pre-checking, and (2) a lack of human readable errors when failing those checks. Regarding (2), I agree slightly -- for methods that use parameters that are user-specified, we should add better messaging that can help remediate. I'm not sure that'll be helpful everywhere though, since some methods are deeper in the call stack and use variables that have already been post-processed in some way. Regarding (1), I'm fine ripping out Preconditions usages. But we can also add some of these messages without getting rid of Objects.requireNonNull (there's a two-arg method for supplying the message), any objection to that? -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
