gerlowskija commented on PR #4264:
URL: https://github.com/apache/solr/pull/4264#issuecomment-4246572454

   > I couldn't help it, I also found some remnants of the trusted configset 
concept
   
   **shakes fist at cloud**
   
   I think you know what I think about bundling that into this PR 😛   You 
definitely get a "pass" though IMO since this PR itself was split off from a 
different one haha
   
   To re-state my concern/issue for others though:
   
   I get that "this should be a separate PR" is really whiny and really 
pedantic.  But in a post-AI world where it's easier than ever to write PRs (but 
no easier to review them) it's worth doing whatever we can to make the review 
side of things easier.  Including being a little more militant about this 
PR-scope-creep stuff.
   
   It's not just a bit of change to the diff - now anyone that wants to do a 
thorough review of this PR has to understand all of the context around trusted 
configsets, why we removed them recently, and whether there was a reason this 
code wasn't included in that removal, etc.
   
   (To save others the work: SOLR-17584 removed "trusted configsets", and it 
was intended to get all the logic, so we're 👍 to remove any vestigial bits 
here.)
   
   I'll get off my soapbox, and I won't ask that you break things up at this 
point.  Mainly hoping I can win hearts and minds on this for the future 😛 


-- 
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]

Reply via email to