I meant "Hi Michael" not Luke. Sorry Michael and Luke. Best, Bruno
On Fri, Jun 19, 2020 at 10:47 AM Bruno Cadonna <br...@confluent.io> wrote: > > Hi Luke, > > The guide is a bit outdated. Thank you for pointing it out. I updated the > guide. > > As Gwen stated above: > > > Unfortunately, you need to get a committer to approve running the tests. > > So, yes a committer has to comment on the PR. > > Best, > Bruno > > On Fri, Jun 19, 2020 at 1:28 AM Michael Carter > <michael.car...@instaclustr.com> wrote: > > > > Hi Gwen and Luke, > > > > Sorry, I’ve probably misunderstood something again. Since KAFKA-10155 and > > KAFKA-10147 have now been resolved, I merged the trunk back into my branch > > and added to comment “retest this please” to my pull request > > (https://github.com/apache/kafka/pull/8844 > > <https://github.com/apache/kafka/pull/8844>) like the contributing > > guidelines state. Unfortunately, no tests seem to have been run. > > Does a committer have to comment on the PR instead? > > > > Thanks, > > Michael > > > > > On 16 Jun 2020, at 9:33 am, Michael Carter > > > <michael.car...@instaclustr.com> wrote: > > > > > > Great, thanks Luke. > > > I’ve undone the patch and added that comment. > > > > > > Cheers, > > > Michael > > > > > >> On 15 Jun 2020, at 6:07 pm, Luke Chen <show...@gmail.com> wrote: > > >> > > >> Hi Michael, > > >> The failed unit test has already handled here: > > >> https://issues.apache.org/jira/browse/KAFKA-10155 > > >> https://issues.apache.org/jira/browse/KAFKA-10147 > > >> > > >> So, maybe you can ignore the test errors and mention the issue number in > > >> PR. > > >> Thanks. > > >> > > >> Luke > > >> > > >> On Mon, Jun 15, 2020 at 3:23 PM Michael Carter < > > >> michael.car...@instaclustr.com> wrote: > > >> > > >>> Thanks for the response Gwen, that clarifies things for me. > > >>> > > >>> Regarding the unit test (ReassignPartitionsUnitTest. > > >>> testModifyBrokerThrottles), it appears to fail quite reliably on trunk > > >>> as > > >>> well (at least on my machine). > > >>> It looks to me like a new override to > > >>> MockAdminClient.describeConfigs(Collection<ConfigResource> resources) > > >>> (MockAdminClient.java line 369) introduced in commit > > >>> 48b56e533b3ff22ae0e2cf7fcc649e7df19f2b06 changed the behaviour of this > > >>> method that the unit test relied on. > > >>> I’ve just now put a patch into my branch to make that test pass by > > >>> calling > > >>> a slightly different version of describeConfigs (that avoids the > > >>> overridden > > >>> behaviour). It’s probably arguable whether that constitutes a fix or not > > >>> though. > > >>> > > >>> Cheers, > > >>> Michael > > >>> > > >>>> On 15 Jun 2020, at 3:41 pm, Gwen Shapira <g...@confluent.io> wrote: > > >>>> > > >>>> Hi, > > >>>> > > >>>> 1. Unfortunately, you need to get a committer to approve running the > > >>> tests. > > >>>> I just gave the green-light on your PR. > > >>>> 2. You can hope that committers will see your PR, but sometimes things > > >>> get > > >>>> lost. If you know someone who is familiar with that area of the code, > > >>>> it > > >>> is > > >>>> a good idea to ping them. > > >>>> 3. We do have some flaky tests. You can see that Jenkins will run 3 > > >>>> parallel builds, if some of them pass and the committer confirms that > > >>>> failures are not related to your code, we are ok to merge. Obviously, > > >>>> if > > >>>> you end up tracking them down and fixing, everyone will be very > > >>>> grateful. > > >>>> > > >>>> Hope this helps, > > >>>> > > >>>> Gwen > > >>>> > > >>>> On Sun, Jun 14, 2020 at 5:52 PM Michael Carter < > > >>>> michael.car...@instaclustr.com> wrote: > > >>>> > > >>>>> Hi all, > > >>>>> > > >>>>> I’ve submitted a patch for the first time( > > >>>>> https://github.com/apache/kafka/pull/8844 < > > >>>>> https://github.com/apache/kafka/pull/8844>), and I have a couple of > > >>>>> questions that I’m hoping someone can help me answer. > > >>>>> > > >>>>> I’m a little unclear what happens after that patch has been submitted. > > >>> The > > >>>>> coding guidelines say Jenkins will run tests automatically, but I > > >>>>> don’t > > >>> see > > >>>>> any results anywhere. Have I misunderstood what should happen, or do I > > >>> just > > >>>>> not know where to look? > > >>>>> Should I be attempting to find reviewers for the change myself, or is > > >>> that > > >>>>> done independently of the patch submitter? > > >>>>> > > >>>>> Also, in resolving a couple of conflicts that have arisen after the > > >>> patch > > >>>>> was first submitted, I noticed that there are now failing unit tests > > >>> that > > >>>>> have nothing to do with my change. Is there a convention on how to > > >>>>> deal > > >>>>> with these? Should it be something that I try to fix on my branch? > > >>>>> > > >>>>> Any thoughts are appreciated. > > >>>>> > > >>>>> Thanks, > > >>>>> Michael > > >>>> > > >>>> > > >>>> > > >>>> -- > > >>>> Gwen Shapira > > >>>> Engineering Manager | Confluent > > >>>> 650.450.2760 | @gwenshap > > >>>> Follow us: Twitter | blog > > >>> > > >>> > > > > >
