Thanks Bruno
> On 19 Jun 2020, at 6:48 pm, Bruno Cadonna <br...@confluent.io> wrote:
>
> 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
>>>>>>
>>>>>>
>>>>
>>>
