Thanks Jack for the updates. Some more feedback:
1) It would be better if you can add the Javadoc in the Public interfaces section. That is a general practice used which gives the readers of the KIP a high level idea of the Public Interfaces. 2) In the proposed section, the bit about marking headers as read only seems like an implementation detail This can generally be avoided in KIPs. 3) Also, in the Deprecation section, can you mention again that this is a backward compatible change and the reason for it (already done in the Proposed Changes section). 4) In the Testing Plan section, there is still the KIP template bit copied over. That can be removed. Thanks! Sagar. On Thu, Jul 20, 2023 at 2:48 PM Jack Tomy <jacktomy...@gmail.com> wrote: > Hey Everyone, > > Please consider this as a reminder and share your feedback. Thank you. > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937 > > On Tue, Jul 18, 2023 at 5:43 PM Jack Tomy <jacktomy...@gmail.com> wrote: > > > Hey @Sagar, > > > > Thank you for the response and feedback. > > > > 1. Done > > 2. Yeah, that was a mistake from my end. Corrected. > > 3. Can you please elaborate this, I have added the java doc along with > > the code changes. Should I paste the same in KIP too? > > 4. Moved. > > 5. I have added one more use case, it is actually helpful in any > > situation where you want to pass some information to partition method > but > > don't have to have it in the key or value. > > 6. Added. > > > > > > Hey @Sagar and everyone, > > Please have a look at the new version and share your thoughts. > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937 > > > > > > On Tue, Jul 18, 2023 at 9:53 AM Sagar <sagarmeansoc...@gmail.com> wrote: > > > >> Hi Jack, > >> > >> Thanks for the KIP! Seems like an interesting idea. I have some > feedback: > >> > >> 1) It would be great if you could clean up the text that seems to mimic > >> the > >> KIP template. It is generally not required in the KIP. > >> > >> 2) In the Public Interfaces where you mentioned *Partitioner method in > >> **org/apache/kafka/clients/producer > >> will have the following update*, I believe you meant the Partitioner > >> *interface*? > >> > >> 3) Staying on Public Interface, it is generally preferable to add a > >> Javadocs section along with the newly added method. You could also > >> describe > >> the behaviour of it invoking the default existing method. > >> > >> 4) The option that is mentioned in the Rejected Alternatives, seems more > >> like a workaround to the current problem that you are describing. That > >> could be added to the Motivation section IMO. > >> > >> 5) Can you also add some more examples of scenarios where this would be > >> helpful? The only scenario mentioned seems to have a workaround. Just > >> trying to ensure that we have a strong enough motivation before adding a > >> public API. > >> > >> 6) One thing which should also be worth noting down would be what > happens > >> if users override both methods, only one method (new or old) and no > >> methods > >> (the default behaviour). It would help in understanding the proposal > >> better. > >> > >> Thanks! > >> Sagar. > >> > >> > >> On Mon, Jul 17, 2023 at 9:19 PM Jack Tomy <jacktomy...@gmail.com> > wrote: > >> > >> > Hey everyone, > >> > > >> > Not seeing much discussion on the KPI. Might be because it is too > >> > obvious 😉. > >> > > >> > If there are no more comments, I will start the VOTE in the coming > days. > >> > > >> > On Sat, Jul 15, 2023 at 8:48 PM Jack Tomy <jacktomy...@gmail.com> > >> wrote: > >> > > >> > > Hey everyone, > >> > > > >> > > Please take a look at the KPI below and provide your suggestions and > >> > > feedback. TIA. > >> > > > >> > > > >> > > >> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937 > >> > > > >> > > > >> > > -- > >> > > Best Regards > >> > > *Jack* > >> > > > >> > > >> > > >> > -- > >> > Best Regards > >> > *Jack* > >> > > >> > > > > > > -- > > Best Regards > > *Jack* > > > > > -- > Best Regards > *Jack* >
