shubhamvishu commented on PR #12880: URL: https://github.com/apache/lucene/pull/12880#issuecomment-1844826409
Thanks @gsmiller @dweiss for taking a look. I see some great points raised here by both of you(and I agree to all). A couple of points why I think we should go with this change : - Backporting : AFAIK backporting this particular change at least shouldn't be a problem because we are only removing the redundant modifiers here which are not adding any value(maybe other folks might know better if i'm missing something). This was one of the reason I thought this might be quick code cleanup. - Less cognitive load : I think unnecessarily adding these modifiers increases the cognitive load the reader goes through while reading code. Sorry if I'm being subjective but I feel adding public to interface methods for eg just adds extra complexity/confusion and my perception is this particular change simplifies that but Idk if its the opposite for any folks who are not much familiar with the language. - Consistency : I agree some might prefer to have "public static" redundancy as Dawid mentioned but I think should we try to have the same convention to other interfaces as well(currently its a mix of both which I believe only contributes to adding more unnecessary confusion/complexity to code). Maybe we could try to stick to one whichever community feel would be better(like we try to do with false conditionals, `x == false` and not `!x`) and this might be a good start. Though I understand its difficult to enforce such things in fast growing open source packages and not possible or at least not a good idea to automate as mentioned by Dawid but we should try to cleanup if we see some opportunity to simply things. If we believe its ok to leave it as is on the individual preference, I'm completely fine with that too but personally I'd be in favour of this one(if not the general cleanup changes which involve other complexities sometimes). -- 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...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
